On Thu, May 30, 2019 at 4:55 AM Gen Zhang <blackgod016574@xxxxxxxxx> wrote: > > In selinux_add_mnt_opt(), 'val' is allcoted by kmemdup_nul(). It returns > NULL when fails. So 'val' should be checked. > > Signed-off-by: Gen Zhang <blackgod016574@xxxxxxxxx> > Fixes: 757cbe597fe8 ("LSM: new method: ->sb_add_mnt_opt()") Previous comments regarding "selinux:" instead of "hooks:" apply here as well. > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 3ec702c..4797c63 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -1052,8 +1052,11 @@ static int selinux_add_mnt_opt(const char *option, const char *val, int len, > if (token == Opt_error) > return -EINVAL; > > - if (token != Opt_seclabel) > - val = kmemdup_nul(val, len, GFP_KERNEL); > + if (token != Opt_seclabel) { > + val = kmemdup_nul(val, len, GFP_KERNEL); > + if (!val) > + return -ENOMEM; It looks like this code is only ever called by NFS, which will eventually clean up mnt_opts via security_free_mnt_opts(), but since the selinux_add_opt() error handler below cleans up mnt_opts it might be safer to do the same here in case this function is called multiple times to add multiple options. > + } > rc = selinux_add_opt(token, val, mnt_opts); > if (unlikely(rc)) { > kfree(val); -- paul moore www.paul-moore.com