Re: [PATCH] selinux: fix handling of empty opts in selinux_fs_context_submount()

[Jeff Layton <jlayton@kernel.org>]

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>