| Message ID | 20230911142358.883728-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux: fix handling of empty opts in selinux_fs_context_submount() | expand |
On Mon, 2023-09-11 at 16:23 +0200, Ondrej Mosnacek wrote: > selinux_set_mnt_opts() relies on the fact that the mount options pointer > is always NULL when all options are unset (specifically in its > !selinux_initialized() branch. However, the new > selinux_fs_context_submount() hook breaks this rule by allocating a new > structure even if no options are set. That causes any submount created > before a SELinux policy is loaded to be rejected in > selinux_set_mnt_opts(). > > Fix this by making selinux_fs_context_submount() leave fc->security > set to NULL when there are no options to be copied from the reference > superblock. > > Reported-by: Adam Williamson <awilliam@redhat.com> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2236345 > Fixes: d80a8f1b58c2 ("vfs, security: Fix automount superblock LSM init problem, preventing NFS sb sharing") > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > security/selinux/hooks.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 10350534de6d6..2aa0e219d7217 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2775,14 +2775,20 @@ static int selinux_umount(struct vfsmount *mnt, int flags) > static int selinux_fs_context_submount(struct fs_context *fc, > struct super_block *reference) > { > - const struct superblock_security_struct *sbsec; > + const struct superblock_security_struct *sbsec = selinux_superblock(reference); > struct selinux_mnt_opts *opts; > > + /* > + * Ensure that fc->security remains NULL when no options are set > + * as expected by selinux_set_mnt_opts(). > + */ > + if (!(sbsec->flags & (FSCONTEXT_MNT|CONTEXT_MNT|DEFCONTEXT_MNT))) > + return 0; > + > opts = kzalloc(sizeof(*opts), GFP_KERNEL); > if (!opts) > return -ENOMEM; > > - sbsec = selinux_superblock(reference); > if (sbsec->flags & FSCONTEXT_MNT) > opts->fscontext_sid = sbsec->sid; > if (sbsec->flags & CONTEXT_MNT) Reviewed-by: Jeff Layton <jlayton@kernel.org>
On Sep 11, 2023 Ondrej Mosnacek <omosnace@redhat.com> wrote: > > selinux_set_mnt_opts() relies on the fact that the mount options pointer > is always NULL when all options are unset (specifically in its > !selinux_initialized() branch. However, the new > selinux_fs_context_submount() hook breaks this rule by allocating a new > structure even if no options are set. That causes any submount created > before a SELinux policy is loaded to be rejected in > selinux_set_mnt_opts(). > > Fix this by making selinux_fs_context_submount() leave fc->security > set to NULL when there are no options to be copied from the reference > superblock. > > Reported-by: Adam Williamson <awilliam@redhat.com> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2236345 > Fixes: d80a8f1b58c2 ("vfs, security: Fix automount superblock LSM init problem, preventing NFS sb sharing") > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > Reviewed-by: Jeff Layton <jlayton@kernel.org> > --- > security/selinux/hooks.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) Thanks Ondrej, this looks good. I'm going to merge this into selinux/stable-6.6 and assuming all goes well with the automated testing (I can't imagine it would catch anything) I'll send this up to Linus later this week. I'm also tagging this for the stable kernels even though this patch is only present in v6.6-rc1 because the original patch has a number of 'Fixes:' tags which means the stable folks will probably end up pulling it into their trees. -- paul-moore.com
On Mon, Sep 11, 2023 at 04:23:58PM +0200, Ondrej Mosnacek wrote: > selinux_set_mnt_opts() relies on the fact that the mount options pointer > is always NULL when all options are unset (specifically in its > !selinux_initialized() branch. However, the new > selinux_fs_context_submount() hook breaks this rule by allocating a new > structure even if no options are set. That causes any submount created > before a SELinux policy is loaded to be rejected in > selinux_set_mnt_opts(). > > Fix this by making selinux_fs_context_submount() leave fc->security > set to NULL when there are no options to be copied from the reference > superblock. > > Reported-by: Adam Williamson <awilliam@redhat.com> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2236345 > Fixes: d80a8f1b58c2 ("vfs, security: Fix automount superblock LSM init problem, preventing NFS sb sharing") > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- Looks good to me, Reviewed-by: Christian Brauner <brauner@kernel.org>
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 10350534de6d6..2aa0e219d7217 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2775,14 +2775,20 @@ static int selinux_umount(struct vfsmount *mnt, int flags) static int selinux_fs_context_submount(struct fs_context *fc, struct super_block *reference) { - const struct superblock_security_struct *sbsec; + const struct superblock_security_struct *sbsec = selinux_superblock(reference); struct selinux_mnt_opts *opts; + /* + * Ensure that fc->security remains NULL when no options are set + * as expected by selinux_set_mnt_opts(). + */ + if (!(sbsec->flags & (FSCONTEXT_MNT|CONTEXT_MNT|DEFCONTEXT_MNT))) + return 0; + opts = kzalloc(sizeof(*opts), GFP_KERNEL); if (!opts) return -ENOMEM; - sbsec = selinux_superblock(reference); if (sbsec->flags & FSCONTEXT_MNT) opts->fscontext_sid = sbsec->sid; if (sbsec->flags & CONTEXT_MNT)
selinux_set_mnt_opts() relies on the fact that the mount options pointer is always NULL when all options are unset (specifically in its !selinux_initialized() branch. However, the new selinux_fs_context_submount() hook breaks this rule by allocating a new structure even if no options are set. That causes any submount created before a SELinux policy is loaded to be rejected in selinux_set_mnt_opts(). Fix this by making selinux_fs_context_submount() leave fc->security set to NULL when there are no options to be copied from the reference superblock. Reported-by: Adam Williamson <awilliam@redhat.com> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2236345 Fixes: d80a8f1b58c2 ("vfs, security: Fix automount superblock LSM init problem, preventing NFS sb sharing") Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- security/selinux/hooks.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)