Messages in this thread | | | From | Wenwen Wang <> | Date | Thu, 15 Aug 2019 16:05:47 -0400 | Subject | Re: [PATCH] wimax/i2400m: fix a memory leak bug |
| |
On Thu, Aug 15, 2019 at 2:45 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote: > > * Wenwen Wang <wenwen@cs.uga.edu> [190815 14:05]: > > In i2400m_barker_db_init(), 'options_orig' is allocated through kstrdup() > > to hold the original command line options. Then, the options are parsed. > > However, if an error occurs during the parsing process, 'options_orig' is > > not deallocated, leading to a memory leak bug. To fix this issue, free > > 'options_orig' before returning the error. > > > > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> > > --- > > drivers/net/wimax/i2400m/fw.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c > > index e9fc168..6b36f6d 100644 > > --- a/drivers/net/wimax/i2400m/fw.c > > +++ b/drivers/net/wimax/i2400m/fw.c > > @@ -342,6 +342,7 @@ int i2400m_barker_db_init(const char *_options) > > "a 32-bit number\n", > > __func__, token); > > result = -EINVAL; > > + kfree(options_orig); > > goto error_parse; > > } > > if (barker == 0) { > > @@ -350,8 +351,10 @@ int i2400m_barker_db_init(const char *_options) > > continue; > > } > > result = i2400m_barker_db_add(barker); > > - if (result < 0) > > + if (result < 0) { > > + kfree(options_orig); > > goto error_add; > > I know that you didn't add this error_add label, but it seems like the > incorrect goto label. Although looking at the caller indicates an add > failed, this label is used prior to and after the memory leak you are > trying to fix. It might be better to change this label to something > like error_parse_add and move the kfree to the unwinding. If a new > label is used, it becomes more clear as to what is being undone and > there aren't two jumps into an unwind from two very different stages of > the function. Adding a new label also has the benefit of moving the > kfree to the unwind of error_parse.
Thanks for your suggestion! I will rework the patch.
Wenwen
| |