| Message ID | 20190612132821.GA3816@zhanggen-UX430UQ (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v4] selinux: fix a missing-check bug in selinux_add_mnt_opt( ) | expand |
On Wed, Jun 12, 2019 at 9:28 AM Gen Zhang <blackgod016574@gmail.com> wrote: > In selinux_add_mnt_opt(), 'val' is allocated by kmemdup_nul(). It returns > NULL when fails. So 'val' should be checked. And 'mnt_opts' should be > freed when error. > > Signed-off-by: Gen Zhang <blackgod016574@gmail.com> > Fixes: 757cbe597fe8 ("LSM: new method: ->sb_add_mnt_opt()") > --- > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 3ec702c..b4b888e 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -1052,15 +1052,24 @@ 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) { > + rc = -ENOMEM; > + goto free_opt; > + } This block has an extra indent, but I'll go ahead and fix that during the merge. I have to mention that had you run this patch through ./scripts/checkpatch.pl it would have caught the mistake :) Regardless, thanks for the patch, I've merged it into the selinux/stable-5.2 branch and I'll be testing it shortly. > + } > rc = selinux_add_opt(token, val, mnt_opts); > if (unlikely(rc)) { > kfree(val); > - if (*mnt_opts) { > - selinux_free_mnt_opts(*mnt_opts); > - *mnt_opts = NULL; > - } > + goto free_opt; > + } > + return rc; > + > +free_opt: > + if (*mnt_opts) { > + selinux_free_mnt_opts(*mnt_opts); > + *mnt_opts = NULL; > } > return rc; > }
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 3ec702c..b4b888e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1052,15 +1052,24 @@ 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) { + rc = -ENOMEM; + goto free_opt; + } + } rc = selinux_add_opt(token, val, mnt_opts); if (unlikely(rc)) { kfree(val); - if (*mnt_opts) { - selinux_free_mnt_opts(*mnt_opts); - *mnt_opts = NULL; - } + goto free_opt; + } + return rc; + +free_opt: + if (*mnt_opts) { + selinux_free_mnt_opts(*mnt_opts); + *mnt_opts = NULL; } return rc; }
In selinux_add_mnt_opt(), 'val' is allocated by kmemdup_nul(). It returns NULL when fails. So 'val' should be checked. And 'mnt_opts' should be freed when error. Signed-off-by: Gen Zhang <blackgod016574@gmail.com> Fixes: 757cbe597fe8 ("LSM: new method: ->sb_add_mnt_opt()") ---