Hello Alejandro!
On Fri, Apr 14, 2023 at 06:35:22PM +0200, Alejandro Colomar wrote:
> On 4/14/23 17:59, Günther Noack wrote:
> > Signed-off-by: Günther Noack <gnoack3000@xxxxxxxxx>
> > ---
> > man7/landlock.7 | 70 ++++++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 66 insertions(+), 4 deletions(-)
> >
> > diff --git a/man7/landlock.7 b/man7/landlock.7
> > index 24488465e..64bfa0752 100644
> > --- a/man7/landlock.7
> > +++ b/man7/landlock.7
> > [...]
> > +/*
> > + * Table of available file system access rights by ABI version,
> > + * numbers hardcoded to keep the example short.
> > + */
> > +__u64 landlock_fs_access_rights[] = {
> > + (1ULL << 13) \- 1, /* ABI v1 */
> > + (1ULL << 14) \- 1, /* ABI v2: add "refer" */
> > + (1ULL << 15) \- 1, /* ABI v3: add "truncate" */
> > +};
> > +
> > +int abi = landlock_create_ruleset(NULL, 0,
> > + LANDLOCK_CREATE_RULESET_VERSION);
> > +if (abi <= 0) {
> > + perror("Giving up \- No Landlock support");
>
> Using perror(3) will already print "Operation not supported", since
> errno is ENOTSUP. Maybe this string is redundant? How about the
> following?
>
> perror("landlock_create_ruleset"); // EOPNOTSUPP
The fallback code assumes that we don't know the kernel that we run on,
so in practice we also have to handle ENOSYS.
See https://docs.kernel.org/userspace-api/landlock.html#landlock-abi-versions
I'd suggest to just make it more explicit here that it can be two
different error codes:
if (abi <= 0) {
/* ENOTSUP or ENOSYS */
perror("Giving up \- No Landlock support");
}
Does that sound reasonable?
> BTW, now I checked that while in Linux ENOTSUP and EOPNOTSUPP are
> equivalent, in POSIX the latter has a connotation that it's about
> sockets. Should we document ENOTSUP in landlock_create_ruleset(2)
> instead of EOPNOTSUPP?
EOPNOTSUP is also used in Landlock's kernel documentation,
we'd maybe have to update it there as well.
I'll have a look at what is more common.
> > + exit(EXIT_FAILURE);
> > +}
> > +if (abi > 3)
> > + abi = 3;
>
> This makes the example a line shorter (see MIN(3)):
>
> abi = MIN(abi, 3);
Thanks, good point! I'll add that.
–Günther
