Re: [PATCH v7 4/9] landlock: Add IOCTL access right

["Günther Noack" <gnoack@google.com>]

Hello Jeff!

On Fri, Dec 01, 2023 at 11:51:16AM -0800, Jeff Xu wrote:
> On Fri, Dec 1, 2023 at 6:40 AM Günther Noack <gnoack@google.com> wrote:
> > --- a/security/landlock/limits.h
> > +++ b/security/landlock/limits.h
> > @@ -18,7 +18,10 @@
> >  #define LANDLOCK_MAX_NUM_LAYERS                16
> >  #define LANDLOCK_MAX_NUM_RULES         U32_MAX
> >
> > -#define LANDLOCK_LAST_ACCESS_FS                LANDLOCK_ACCESS_FS_TRUNCATE
> > +#define LANDLOCK_LAST_PUBLIC_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL
> 
> iiuc, for the next feature, it only needs to update
> LANDLOCK_LAST_PUBLIC_ACCESS_FS to the new LANDLOCK_ACCESS_FS_ABC here.
> and keep below the same, right ?
> 
> > +#define LANDLOCK_MASK_PUBLIC_ACCESS_FS ((LANDLOCK_LAST_PUBLIC_ACCESS_FS << 1) - 1)
> > +
> > +#define LANDLOCK_LAST_ACCESS_FS                (LANDLOCK_LAST_PUBLIC_ACCESS_FS << 4)
> maybe add a comment why "<<4" is used ?

I'll add a comment to the section explaining it:

  For file system access rights, Landlock distinguishes between the publicly
  visible access rights (1 to LANDLOCK_LAST_PUBLIC_ACCESS_FS) and the private
  ones which are not exposed to userspace (LANDLOCK_LAST_PUBLIC_ACCESS_FS + 1 to
  LANDLOCK_LAST_ACCESS_FS).  The private access rights are defined in fs.c.

This should clarify both questions, I hope.

You are correct -- the private access rights in fs.c are defined relative to the
last public access right.

—Günther