Re: [PATCH v13 10/10] fs/ioctl: Add a comment to keep the logic in sync with the Landlock LSM

From: Paul Moore <paul@paul-moore.com>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: "Günther Noack" <gnoack@google.com>,
	linux-security-module@vger.kernel.org,
	"Jeff Xu" <jeffxu@google.com>, "Arnd Bergmann" <arnd@arndb.de>,
	"Jorge Lucangeli Obes" <jorgelo@chromium.org>,
	"Allen Webb" <allenwebb@google.com>,
	"Dmitry Torokhov" <dtor@google.com>,
	"Konstantin Meskhidze" <konstantin.meskhidze@huawei.com>,
	"Matt Bobrowski" <repnop@google.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v13 10/10] fs/ioctl: Add a comment to keep the logic in sync with the Landlock LSM
Date: Thu, 28 Mar 2024 13:06:57 -0400	[thread overview]
Message-ID: <CAHC9VhS=ESSCK6TiTXbqDmvDUwPa3UFBN7ZaJogmj=Qguc_m2w@mail.gmail.com> (raw)
In-Reply-To: <20240328.mahn4seChaej@digikod.net>

On Thu, Mar 28, 2024 at 12:43 PM Mickaël Salaün <mic@digikod.net> wrote:
> On Thu, Mar 28, 2024 at 09:08:13AM -0400, Paul Moore wrote:
> > On Thu, Mar 28, 2024 at 8:11 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > On Wed, Mar 27, 2024 at 01:10:40PM +0000, Günther Noack wrote:
> > > > Landlock's IOCTL support needs to partially replicate the list of
> > > > IOCTLs from do_vfs_ioctl().  The list of commands implemented in
> > > > do_vfs_ioctl() should be kept in sync with Landlock's IOCTL policies.
> > > >
> > > > Signed-off-by: Günther Noack <gnoack@google.com>
> > > > ---
> > > >  fs/ioctl.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > > > index 1d5abfdf0f22..661b46125669 100644
> > > > --- a/fs/ioctl.c
> > > > +++ b/fs/ioctl.c
> > > > @@ -796,6 +796,9 @@ static int ioctl_get_fs_sysfs_path(struct file *file, void __user *argp)
> > > >   *
> > > >   * When you add any new common ioctls to the switches above and below,
> > > >   * please ensure they have compatible arguments in compat mode.
> > > > + *
> > > > + * The commands which are implemented here should be kept in sync with the IOCTL
> > > > + * security policies in the Landlock LSM.
> > >
> > > Suggestion:
> > > "with the Landlock IOCTL security policy defined in security/landlock/fs.c"
> >
> > We really shouldn't have any comments or code outside of the security/
> > directory that reference a specific LSM implementation.  I'm sure
> > there are probably a few old comments referring to SELinux, but those
> > are bugs as far as I'm concerned (if anyone spots one, please let me
> > know or send me a patch!).
> >
> > How about the following?
> >
> > "The LSM list should also be notified of any command additions or
>
> "The LSM mailing list..."

 ;)

> > changes as specific LSMs may be affected."
>
> Looks good.

-- 
paul-moore.com