Bug 275330 - Capsicum should have "static" initializer as well
Summary: Capsicum should have "static" initializer as well
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 14.0-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Mariusz Zaborski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-25 12:19 UTC by vini.ipsmaker
Modified: 2024-06-05 16:13 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description vini.ipsmaker 2023-11-25 12:19:58 UTC
I recently wrote Lua bindings for Capsicum and faced two inconveniences with the API.

First problem is related to initalization. I'm using gperf to translate a string known only at runtime (whatever the Lua code is trying to do) to a cap_rights_t. Something like:

cap_rights_t values[N] = {
  CAP_READ, // HASH("CAP_READ") = 0
  CAP_WRITE, // HASH("CAP_WRITE") = 0
  // ...
};

However there are several holes in this table that are used whenever the known-at-runtime-string doesn't point to a valid capability and I need to initialize them to a default value.

Other APIs such as POSIX threads solve this problem by offering a macro (PTHREAD_MUTEX_INITIALIZER) that can be used to initialize the variable instead of calling a function.

It'd be very helpful if Capsicum offered a macro CAP_RIGHTS_INITIALIZER in the same fashion. That'd solve half of my problems with Capsicum's API.

The second problem I'm facing is a lack of a function to check whether cap_rights_t is empty. Right now I have to do this:

cap_rights_t all_rights;
cap_rights_init(&all_rights);
CAP_ALL(&all_rights); //< I don't even know whether CAP_ALL() is part of the intended public API
cap_rights_t complement = all_rights;
cap_rights_remove(&complement, &flag); //< flag is what I want to know whether is empty
if (cap_rights_contains(&complement, &all_rights)) {
    // ...
}

That's *very* inconvenient (and inefficient). Can we have cap_rights_is_empty(flag)? That'd solve the other half of my problems with the API.
Comment 1 Mariusz Zaborski freebsd_committer freebsd_triage 2023-11-25 15:08:12 UTC
Unfortunately, you can't do it with a simple macro. The capability structure is a little bit complex - https://people.freebsd.org/~pjd/pubs/Capsicum_and_Casper.pdf (slides 4-8).

I can look into adding cap_rights_empty, this seems reasonable.
Comment 2 vini.ipsmaker 2023-11-30 09:52:27 UTC
For reference, this is the workaround I'm using for the lack of CAP_RIGHTS_INITIALIZER: https://gitlab.com/emilua/emilua/-/blob/4e4c55eff7676476530032aa2aa424a62c73aeed/src/file_descriptor.cpp#L46 (i.e. C++ features to define a function in-place and call it immediately)

> I can look into adding cap_rights_empty, this seems reasonable.

This means I could replace empty_rights for cap_rights_empty in my code base: https://gitlab.com/emilua/emilua/-/blob/4e4c55eff7676476530032aa2aa424a62c73aeed/src/file_descriptor.cpp#L242

It works for me.

However a function such as cap_rights_is_empty() still would be very helpful to me. As I mentioned before, here's the ugly workaround I've been forced to use: https://gitlab.com/emilua/emilua/-/blob/4e4c55eff7676476530032aa2aa424a62c73aeed/src/file_descriptor.cpp#L349
Comment 3 Mariusz Zaborski freebsd_committer freebsd_triage 2023-11-30 09:53:55 UTC
I have created a PR for cap_rights_is_empty:

https://reviews.freebsd.org/D42780
Comment 4 commit-hook freebsd_committer freebsd_triage 2023-12-11 11:18:09 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a7100ae23aca07976926bd8d50223c45149f65d6

commit a7100ae23aca07976926bd8d50223c45149f65d6
Author:     Mariusz Zaborski <oshogbo@FreeBSD.org>
AuthorDate: 2023-12-11 11:09:31 +0000
Commit:     Mariusz Zaborski <oshogbo@FreeBSD.org>
CommitDate: 2023-12-11 11:15:46 +0000

    capsicum: introduce cap_rights_is_empty Function

    Before this commit, we only had the capability to check if a specific
    capability was set (using cap_rights_is_set function). However, there
    was no efficient method to determine if a cap_rights_t structure doesn't
    contain any capability. The cap_rights_is_empty function addresses
    this gap.

    PR:             275330
    Reported by:    vini.ipsmaker@gmail.com
    Reviewed by:    emaste, markj
    Differential Revision:  https://reviews.freebsd.org/D42780

 contrib/capsicum-test/capability-fd.cc | 15 +++++++++++++++
 lib/libc/capability/Symbol.map         |  4 ++++
 lib/libc/capability/cap_rights_init.3  | 19 ++++++++++++++++++-
 sys/kern/subr_capability.c             | 19 +++++++++++++++++++
 sys/sys/capsicum.h                     |  2 ++
 5 files changed, 58 insertions(+), 1 deletion(-)
Comment 5 vini.ipsmaker 2024-02-12 11:38:11 UTC
> I can look into adding cap_rights_empty

Are you still going to add this symbol?

For my C++ programs, I can use a workaround that is good enough: https://gitlab.com/emilua/emilua/-/blob/4e4c55eff7676476530032aa2aa424a62c73aeed/src/file_descriptor.cpp#L46 (IIFE)

For my C++ programs, I will never miss cap_rights_empty.

However what about C programmers? Capsicum is a C API after all. Honestly I don't really code in pure C that much nowadays, so I'm not in the best position to judge. What do you think?
Comment 6 commit-hook freebsd_committer freebsd_triage 2024-06-05 16:13:37 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e77813f7e4a32be5d0278c5cb38bcc0a759025d0

commit e77813f7e4a32be5d0278c5cb38bcc0a759025d0
Author:     Mariusz Zaborski <oshogbo@FreeBSD.org>
AuthorDate: 2023-12-11 11:09:31 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2024-06-05 15:55:17 +0000

    capsicum: introduce cap_rights_is_empty Function

    Before this commit, we only had the capability to check if a specific
    capability was set (using cap_rights_is_set function). However, there
    was no efficient method to determine if a cap_rights_t structure doesn't
    contain any capability. The cap_rights_is_empty function addresses
    this gap.

    PR:             275330
    Reported by:    vini.ipsmaker@gmail.com
    Reviewed by:    emaste, markj
    Differential Revision:  https://reviews.freebsd.org/D42780

    (cherry picked from commit a7100ae23aca07976926bd8d50223c45149f65d6)

 contrib/capsicum-test/capability-fd.cc | 15 +++++++++++++++
 lib/libc/capability/Symbol.map         |  4 ++++
 lib/libc/capability/cap_rights_init.3  | 19 ++++++++++++++++++-
 sys/kern/subr_capability.c             | 19 +++++++++++++++++++
 sys/sys/capsicum.h                     |  2 ++
 5 files changed, 58 insertions(+), 1 deletion(-)