* [Memory leak]: memory leak in apply_r_mips_lo16_rel @ 2012-08-03 22:29 Akhilesh Kumar 2012-08-03 22:29 ` Akhilesh Kumar 2012-08-08 13:04 ` Ralf Baechle 0 siblings, 2 replies; 4+ messages in thread From: Akhilesh Kumar @ 2012-08-03 22:29 UTC (permalink / raw) To: ralf; +Cc: paul.gortmaker, linux-mips [-- Attachment #1: Type: text/plain, Size: 1120 bytes --] Hi Ralf, I found some memory leak in arch/mips/kernel/module.c file Please review below patch and share your review comments, Thanks, Akhilesh >From 77b8cae374a95000a1fd7e75bcda6694b8180fe9 Mon Sep 17 00:00:00 2001 From: Akhilesh Kumar <akhilesh.lxr@gmail.com> Date: Sat, 4 Aug 2012 03:34:06 +0530 Subject: [Memory leak]: memory leak in apply_r_mips_lo16_rel module.c if (v != l->value) goto out_danger ; out_danger: pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); return -ENOEXEC; in case goto_out_danger kfree(l) is missing Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> --- arch/mips/kernel/module.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c index a5066b1..b1dce44 100644 --- a/arch/mips/kernel/module.c +++ b/arch/mips/kernel/module.c @@ -202,7 +202,7 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) out_danger: pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); - + kfree(l); return -ENOEXEC; } -- 1.7.8.4 [-- Attachment #2: Type: text/html, Size: 1930 bytes --] ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Memory leak]: memory leak in apply_r_mips_lo16_rel 2012-08-03 22:29 [Memory leak]: memory leak in apply_r_mips_lo16_rel Akhilesh Kumar @ 2012-08-03 22:29 ` Akhilesh Kumar 2012-08-08 13:04 ` Ralf Baechle 1 sibling, 0 replies; 4+ messages in thread From: Akhilesh Kumar @ 2012-08-03 22:29 UTC (permalink / raw) To: ralf; +Cc: paul.gortmaker, linux-mips [-- Attachment #1: Type: text/plain, Size: 1119 bytes --] Hi Ralf, I found some memory leak in arch/mips/kernel/module.c file Please review below patch and share your review comments, Thanks, Akhilesh From 77b8cae374a95000a1fd7e75bcda6694b8180fe9 Mon Sep 17 00:00:00 2001 From: Akhilesh Kumar <akhilesh.lxr@gmail.com> Date: Sat, 4 Aug 2012 03:34:06 +0530 Subject: [Memory leak]: memory leak in apply_r_mips_lo16_rel module.c if (v != l->value) goto out_danger ; out_danger: pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); return -ENOEXEC; in case goto_out_danger kfree(l) is missing Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> --- arch/mips/kernel/module.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c index a5066b1..b1dce44 100644 --- a/arch/mips/kernel/module.c +++ b/arch/mips/kernel/module.c @@ -202,7 +202,7 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) out_danger: pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); - + kfree(l); return -ENOEXEC; } -- 1.7.8.4 [-- Attachment #2: Type: text/html, Size: 1930 bytes --] ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Memory leak]: memory leak in apply_r_mips_lo16_rel 2012-08-03 22:29 [Memory leak]: memory leak in apply_r_mips_lo16_rel Akhilesh Kumar 2012-08-03 22:29 ` Akhilesh Kumar @ 2012-08-08 13:04 ` Ralf Baechle 2012-08-08 13:04 ` Ralf Baechle 1 sibling, 1 reply; 4+ messages in thread From: Ralf Baechle @ 2012-08-08 13:04 UTC (permalink / raw) To: Akhilesh Kumar; +Cc: paul.gortmaker, linux-mips On Sat, Aug 04, 2012 at 03:59:50AM +0530, Akhilesh Kumar wrote: > > I found some memory leak in > arch/mips/kernel/module.c file > > Please review below patch and share your review comments, > > Thanks, > Akhilesh > > > >From 77b8cae374a95000a1fd7e75bcda6694b8180fe9 Mon Sep 17 00:00:00 2001 > From: Akhilesh Kumar <akhilesh.lxr@gmail.com> > Date: Sat, 4 Aug 2012 03:34:06 +0530 > Subject: [Memory leak]: memory leak in apply_r_mips_lo16_rel > module.c > > if (v != l->value) > goto out_danger ; > out_danger: > pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); > return -ENOEXEC; > > in case goto_out_danger kfree(l) is missing > > Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> > --- > arch/mips/kernel/module.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c > index a5066b1..b1dce44 100644 > --- a/arch/mips/kernel/module.c > +++ b/arch/mips/kernel/module.c > @@ -202,7 +202,7 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 > *location, Elf_Addr v) > > out_danger: > pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); > - > + kfree(l); The variable l isn't declared at this point. You obviously haven't tried to compile this. > return -ENOEXEC; Well spotted - this bug has been around for ages. The fix is incorrect though. L is pointing to a linked list and we need to free the entire linked list, not just the element currently being processed. I noticed the same issue in VPE loader in arch/mips/kernel/vpe.c, function apply_r_mips_lo16() and fixed it in 477c4b07406357ad93d0e32788dbf3ee814eadaa / 6f5d2e970452b5c86906adcb8e7ad246f535ba39 [MIPS: VPE: Free relocation chain on error.] but back then almost exactly 3 years ago I did not notice the same issue to exist in the module loader as well. Also reviewing the HI16/LO16 processing I noticed that there is a race condition if multiple modules are being loaded in parallel - but that's a different problem. Ralf >From 3ae0244ccd4cd56293fd5764aaf30127882a0170 Mon Sep 17 00:00:00 2001 From: Ralf Baechle <ralf@linux-mips.org> Date: Wed, 8 Aug 2012 14:57:03 +0200 Subject: [PATCH] MIPS: Fix memory leak in error path of HI16/LO16 relocation handling. Commit 6f5d2e970452b5c86906adcb8e7ad246f535ba39 (lmo) / 477c4b07406357ad93d0e32788dbf3ee814eadaa (kernel.org) [[MIPS: VPE: Free relocation chain on error.] fixed the same issue in the vpe loader in 2009 but back then the same bug in module.c went unfixed. Signed-off-by: Ralf Baechle <ralf@linux-mips.org> Reported-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> --- arch/mips/kernel/module.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c index a5066b1..e5f2f56 100644 --- a/arch/mips/kernel/module.c +++ b/arch/mips/kernel/module.c @@ -146,16 +146,15 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) { unsigned long insnlo = *location; Elf_Addr val, vallo; + struct mips_hi16 *l, *next; /* Sign extend the addend we extract from the lo insn. */ vallo = ((insnlo & 0xffff) ^ 0x8000) - 0x8000; if (mips_hi16_list != NULL) { - struct mips_hi16 *l; l = mips_hi16_list; while (l != NULL) { - struct mips_hi16 *next; unsigned long insn; /* @@ -201,6 +200,12 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) return 0; out_danger: + while (l) { + next = l->next; + kfree(l); + l = next; + } + pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); return -ENOEXEC; ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Memory leak]: memory leak in apply_r_mips_lo16_rel 2012-08-08 13:04 ` Ralf Baechle @ 2012-08-08 13:04 ` Ralf Baechle 0 siblings, 0 replies; 4+ messages in thread From: Ralf Baechle @ 2012-08-08 13:04 UTC (permalink / raw) To: Akhilesh Kumar; +Cc: paul.gortmaker, linux-mips On Sat, Aug 04, 2012 at 03:59:50AM +0530, Akhilesh Kumar wrote: > > I found some memory leak in > arch/mips/kernel/module.c file > > Please review below patch and share your review comments, > > Thanks, > Akhilesh > > > >From 77b8cae374a95000a1fd7e75bcda6694b8180fe9 Mon Sep 17 00:00:00 2001 > From: Akhilesh Kumar <akhilesh.lxr@gmail.com> > Date: Sat, 4 Aug 2012 03:34:06 +0530 > Subject: [Memory leak]: memory leak in apply_r_mips_lo16_rel > module.c > > if (v != l->value) > goto out_danger ; > out_danger: > pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); > return -ENOEXEC; > > in case goto_out_danger kfree(l) is missing > > Signed-off-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> > --- > arch/mips/kernel/module.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c > index a5066b1..b1dce44 100644 > --- a/arch/mips/kernel/module.c > +++ b/arch/mips/kernel/module.c > @@ -202,7 +202,7 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 > *location, Elf_Addr v) > > out_danger: > pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); > - > + kfree(l); The variable l isn't declared at this point. You obviously haven't tried to compile this. > return -ENOEXEC; Well spotted - this bug has been around for ages. The fix is incorrect though. L is pointing to a linked list and we need to free the entire linked list, not just the element currently being processed. I noticed the same issue in VPE loader in arch/mips/kernel/vpe.c, function apply_r_mips_lo16() and fixed it in 477c4b07406357ad93d0e32788dbf3ee814eadaa / 6f5d2e970452b5c86906adcb8e7ad246f535ba39 [MIPS: VPE: Free relocation chain on error.] but back then almost exactly 3 years ago I did not notice the same issue to exist in the module loader as well. Also reviewing the HI16/LO16 processing I noticed that there is a race condition if multiple modules are being loaded in parallel - but that's a different problem. Ralf From 3ae0244ccd4cd56293fd5764aaf30127882a0170 Mon Sep 17 00:00:00 2001 From: Ralf Baechle <ralf@linux-mips.org> Date: Wed, 8 Aug 2012 14:57:03 +0200 Subject: [PATCH] MIPS: Fix memory leak in error path of HI16/LO16 relocation handling. Commit 6f5d2e970452b5c86906adcb8e7ad246f535ba39 (lmo) / 477c4b07406357ad93d0e32788dbf3ee814eadaa (kernel.org) [[MIPS: VPE: Free relocation chain on error.] fixed the same issue in the vpe loader in 2009 but back then the same bug in module.c went unfixed. Signed-off-by: Ralf Baechle <ralf@linux-mips.org> Reported-by: Akhilesh Kumar <akhilesh.lxr@gmail.com> --- arch/mips/kernel/module.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c index a5066b1..e5f2f56 100644 --- a/arch/mips/kernel/module.c +++ b/arch/mips/kernel/module.c @@ -146,16 +146,15 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) { unsigned long insnlo = *location; Elf_Addr val, vallo; + struct mips_hi16 *l, *next; /* Sign extend the addend we extract from the lo insn. */ vallo = ((insnlo & 0xffff) ^ 0x8000) - 0x8000; if (mips_hi16_list != NULL) { - struct mips_hi16 *l; l = mips_hi16_list; while (l != NULL) { - struct mips_hi16 *next; unsigned long insn; /* @@ -201,6 +200,12 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) return 0; out_danger: + while (l) { + next = l->next; + kfree(l); + l = next; + } + pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); return -ENOEXEC; ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-08 13:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-03 22:29 [Memory leak]: memory leak in apply_r_mips_lo16_rel Akhilesh Kumar 2012-08-03 22:29 ` Akhilesh Kumar 2012-08-08 13:04 ` Ralf Baechle 2012-08-08 13:04 ` Ralf Baechle
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox