| Message ID | 20240327131040.158777-11-gnoack@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Landlock: IOCTL support | expand |
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" > */ > static int do_vfs_ioctl(struct file *filp, unsigned int fd, > unsigned int cmd, unsigned long arg) > -- > 2.44.0.396.g6e790dbe36-goog > >
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 changes as specific LSMs may be affected."
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 >
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.
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. */ static int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd, unsigned long arg)
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(+)