| Message ID | 20211202031209.253228-1-bernard@vivo.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | security/selinux: fix potential memleak in error branch | expand |
On Wed, Dec 1, 2021 at 10:12 PM Bernard Zhao <bernard@vivo.com> wrote: > > This patch try to fix potential memleak in error branch. > > Signed-off-by: Bernard Zhao <bernard@vivo.com> > --- > security/selinux/hooks.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 62d30c0a30c2..10700720bb74 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -983,6 +983,7 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, > static int selinux_add_opt(int token, const char *s, void **mnt_opts) > { > struct selinux_mnt_opts *opts = *mnt_opts; > + bool is_alloc_opts = false; > > if (token == Opt_seclabel) /* eaten and completely ignored */ > return 0; > @@ -992,9 +993,13 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) > if (!opts) > return -ENOMEM; > *mnt_opts = opts; > + is_alloc_opts = true; > } > - if (!s) > + if (!s) { > + if (is_alloc_opts) > + kfree(opts); > return -ENOMEM; > + } Thanks for catching this and submitting a patch, but would it be simpler to do the "(!s)" check before the "(!opts)" check? > switch (token) { > case Opt_context: > if (opts->context || opts->defcontext) > @@ -1020,6 +1025,8 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) > return 0; > Einval: > pr_warn(SEL_MOUNT_FAIL_MSG); > + if (is_alloc_opts) > + kfree(opts); > return -EINVAL; > } > > -- > 2.33.1
-----邮件原件----- 发件人: bernard@vivo.com <bernard@vivo.com> 代表 Paul Moore 发送时间: 2021年12月6日 11:23 收件人: 赵军奎 <bernard@vivo.com> 抄送: Stephen Smalley <stephen.smalley.work@gmail.com>; Eric Paris <eparis@parisplace.org>; selinux@vger.kernel.org; linux-kernel@vger.kernel.org 主题: Re: [PATCH] security/selinux: fix potential memleak in error branch On Wed, Dec 1, 2021 at 10:12 PM Bernard Zhao <bernard@vivo.com> wrote: > > This patch try to fix potential memleak in error branch. > > Signed-off-by: Bernard Zhao <bernard@vivo.com> > --- > security/selinux/hooks.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index > 62d30c0a30c2..10700720bb74 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -983,6 +983,7 @@ static int selinux_sb_clone_mnt_opts(const struct > super_block *oldsb, static int selinux_add_opt(int token, const char > *s, void **mnt_opts) { > struct selinux_mnt_opts *opts = *mnt_opts; > + bool is_alloc_opts = false; > > if (token == Opt_seclabel) /* eaten and completely ignored */ > return 0; > @@ -992,9 +993,13 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) > if (!opts) > return -ENOMEM; > *mnt_opts = opts; > + is_alloc_opts = true; > } > - if (!s) > + if (!s) { > + if (is_alloc_opts) > + kfree(opts); > return -ENOMEM; > + } >Thanks for catching this and submitting a patch, but would it be simpler to do the "(!s)" check before the "(!opts)" check? Hi paul moore： This seems to be a good idea, thanks for your comments! I will modify this and resubmit a version of this patch! Thanks! BR//Bernard > switch (token) { > case Opt_context: > if (opts->context || opts->defcontext) @@ -1020,6 > +1025,8 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) > return 0; > Einval: > pr_warn(SEL_MOUNT_FAIL_MSG); > + if (is_alloc_opts) > + kfree(opts); > return -EINVAL; > } > > -- > 2.33.1 -- paul moore www.paul-moore.com
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 62d30c0a30c2..10700720bb74 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -983,6 +983,7 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, static int selinux_add_opt(int token, const char *s, void **mnt_opts) { struct selinux_mnt_opts *opts = *mnt_opts; + bool is_alloc_opts = false; if (token == Opt_seclabel) /* eaten and completely ignored */ return 0; @@ -992,9 +993,13 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) if (!opts) return -ENOMEM; *mnt_opts = opts; + is_alloc_opts = true; } - if (!s) + if (!s) { + if (is_alloc_opts) + kfree(opts); return -ENOMEM; + } switch (token) { case Opt_context: if (opts->context || opts->defcontext) @@ -1020,6 +1025,8 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) return 0; Einval: pr_warn(SEL_MOUNT_FAIL_MSG); + if (is_alloc_opts) + kfree(opts); return -EINVAL; }
This patch try to fix potential memleak in error branch. Signed-off-by: Bernard Zhao <bernard@vivo.com> --- security/selinux/hooks.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)