| Message ID | 20181221090958.GE2735@kadam (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | smack: Fix a memory leak in smack_add_opt() | expand |
On Fri, Dec 21, 2018 at 12:09:58PM +0300, Dan Carpenter wrote: > The function is leaking "opts" on the error paths. > > Fixes: 90e3b564ab93 ("smack: take the guts of smack_parse_opts_str() into a new helper") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> D'oh... Applied, thanks for spotting that braino.
On Fri, Dec 21, 2018 at 04:36:54PM +0000, Al Viro wrote: > On Fri, Dec 21, 2018 at 12:09:58PM +0300, Dan Carpenter wrote: > > The function is leaking "opts" on the error paths. > > > > Fixes: 90e3b564ab93 ("smack: take the guts of smack_parse_opts_str() into a new helper") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > D'oh... Applied, thanks for spotting that braino. On the other hand, it's easier to do it this way - the caller will free the damn thing on error, anyway: diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 11da1e2531c8..cf0c0380e5dd 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -608,6 +608,7 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) opts = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL); if (!opts) return -ENOMEM; + *mnt_opts = opts; } if (!s) return -ENOMEM; @@ -639,7 +640,6 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) opts->fstransmute = s; break; } - *mnt_opts = opts; return 0; out_opt_err:
On 12/21/2018 8:42 AM, Al Viro wrote: > On Fri, Dec 21, 2018 at 04:36:54PM +0000, Al Viro wrote: >> On Fri, Dec 21, 2018 at 12:09:58PM +0300, Dan Carpenter wrote: >>> The function is leaking "opts" on the error paths. >>> >>> Fixes: 90e3b564ab93 ("smack: take the guts of smack_parse_opts_str() into a new helper") >>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >> D'oh... Applied, thanks for spotting that braino. > On the other hand, it's easier to do it this way - the caller will free the damn thing > on error, anyway: What tree/branch is the happening in? > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 11da1e2531c8..cf0c0380e5dd 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -608,6 +608,7 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) > opts = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL); > if (!opts) > return -ENOMEM; > + *mnt_opts = opts; > } > if (!s) > return -ENOMEM; > @@ -639,7 +640,6 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) > opts->fstransmute = s; > break; > } > - *mnt_opts = opts; > return 0; > > out_opt_err: >
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 2adafc1018d9..4e9cdb942677 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -604,13 +604,13 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) { struct smack_mnt_opts *opts = *mnt_opts; + if (!s) + return -ENOMEM; if (!opts) { opts = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL); if (!opts) return -ENOMEM; } - if (!s) - return -ENOMEM; switch (token) { case Opt_fsdefault: @@ -643,6 +643,8 @@ static int smack_add_opt(int token, const char *s, void **mnt_opts) return 0; out_opt_err: + if (opts != *mnt_opts) + kfree(opts); pr_warn("Smack: duplicate mount options\n"); return -EINVAL; }
The function is leaking "opts" on the error paths. Fixes: 90e3b564ab93 ("smack: take the guts of smack_parse_opts_str() into a new helper") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- security/smack/smack_lsm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)