Re: [PATCH] selinux: avoid dereference of garbage after mount failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mar 28, 2024 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@xxxxxxxxxxxxx> wrote:
> 
> In case kern_mount() fails and returns an error pointer return in the
> error branch instead of continuing and dereferencing the error pointer.
> 
> While on it drop the never read static variable selinuxfs_mount.
> 
> Fixes: 0619f0f5e36f ("selinux: wrap selinuxfs state")
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> ---
>  security/selinux/selinuxfs.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index d18581d741e8..7e9aa5d151b4 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -2125,7 +2125,6 @@ static struct file_system_type sel_fs_type = {
>  	.kill_sb	= sel_kill_sb,
>  };
>  
> -static struct vfsmount *selinuxfs_mount __ro_after_init;
>  struct path selinux_null __ro_after_init;
>  
>  static int __init init_sel_fs(void)
> @@ -2147,18 +2146,21 @@ static int __init init_sel_fs(void)
>  		return err;
>  	}
>  
> -	selinux_null.mnt = selinuxfs_mount = kern_mount(&sel_fs_type);
> -	if (IS_ERR(selinuxfs_mount)) {
> +	selinux_null.mnt = kern_mount(&sel_fs_type);
> +	if (IS_ERR(selinux_null.mnt)) {
>  		pr_err("selinuxfs:  could not mount!\n");
> -		err = PTR_ERR(selinuxfs_mount);
> -		selinuxfs_mount = NULL;
> +		err = PTR_ERR(selinux_null.mnt);
> +		selinux_null.mnt = NULL;
> +		return err;
>  	}
> +
>  	selinux_null.dentry = d_hash_and_lookup(selinux_null.mnt->mnt_root,
>  						&null_name);
>  	if (IS_ERR(selinux_null.dentry)) {
>  		pr_err("selinuxfs:  could not lookup null!\n");
>  		err = PTR_ERR(selinux_null.dentry);
>  		selinux_null.dentry = NULL;
> +		return err;

Casey's correct in that we probably don't need this, but it does harden
the source a bit against future changes so it isn't entirely a bad
thing.  If nothing else, some new kernel dev will get excited writing a
oneliner several years from now that removes the redundant return; I'm
getting exited about the patch just thinking about it.

Anyway, thanks for noticing this and submitting a patch Christian,
beyond the above nitpick, everything looks good to me.  I'm going to
merge this into selinux/stable-6.9 with a stable marking and assuming
all goes well I'll send this up to Linus in a few days.

Thanks!

>  	}
>  
>  	return err;
> -- 
> 2.43.0

--
paul-moore.com




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux