| Message ID | 1445545961-5620-1-git-send-email-christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 23, 2015 at 10:37:33AM +0300, Jarkko Sakkinen wrote: > On Thu, Oct 22, 2015 at 10:32:41PM +0200, Christophe JAILLET wrote: > > Reference to the 'np' node is dropped before dereferencing the 'sizep' and > > 'basep' pointers, which could by then point to junk if the node has been > > freed. > > > > Refactor code to call 'of_node_pup' later. > > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > LGTM. Is there anyone able to provide Tested-by for this? Christophe, were you able to reproduce the crash (insmod/rmmod couple of times maybe?) and validate that it was gone after fixing the bug? > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko ------------------------------------------------------------------------------
Le 27/10/2015 11:27, Jarkko Sakkinen a écrit : > On Fri, Oct 23, 2015 at 10:37:33AM +0300, Jarkko Sakkinen wrote: >> On Thu, Oct 22, 2015 at 10:32:41PM +0200, Christophe JAILLET wrote: >>> Reference to the 'np' node is dropped before dereferencing the 'sizep' and >>> 'basep' pointers, which could by then point to junk if the node has been >>> freed. >>> >>> Refactor code to call 'of_node_pup' later. >>> >>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> LGTM. > Is there anyone able to provide Tested-by for this? > > Christophe, were you able to reproduce the crash (insmod/rmmod couple > of times maybe?) and validate that it was gone after fixing the bug? Hi, no, I never triggered the bug. This is just something noticed while looking at potential issues related to incorrect use of 'of_node_pup'. I only compile tested the patch. Best regards, CJ > >> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > /Jarkko > ------------------------------------------------------------------------------
On Thu, Oct 29, 2015 at 07:12:01AM +0100, Marion & Christophe JAILLET wrote: > > > Le 27/10/2015 11:27, Jarkko Sakkinen a écrit : > >On Fri, Oct 23, 2015 at 10:37:33AM +0300, Jarkko Sakkinen wrote: > >>On Thu, Oct 22, 2015 at 10:32:41PM +0200, Christophe JAILLET wrote: > >>>Reference to the 'np' node is dropped before dereferencing the 'sizep' and > >>>'basep' pointers, which could by then point to junk if the node has been > >>>freed. > >>> > >>>Refactor code to call 'of_node_pup' later. > >>> > >>>Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > >>LGTM. > >Is there anyone able to provide Tested-by for this? > > > >Christophe, were you able to reproduce the crash (insmod/rmmod couple > >of times maybe?) and validate that it was gone after fixing the bug? > > Hi, > no, I never triggered the bug. > This is just something noticed while looking at potential issues related to > incorrect use of 'of_node_pup'. > I only compile tested the patch. The fix is so obvious that I see no reason not to include it. Thanks for the good work. > Best regards, > CJ /Jarkko ------------------------------------------------------------------------------
On Thu, Oct 29, 2015 at 12:48:44PM +0200, Jarkko Sakkinen wrote: > On Thu, Oct 29, 2015 at 07:12:01AM +0100, Marion & Christophe JAILLET wrote: > > > > > > Le 27/10/2015 11:27, Jarkko Sakkinen a écrit : > > >On Fri, Oct 23, 2015 at 10:37:33AM +0300, Jarkko Sakkinen wrote: > > >>On Thu, Oct 22, 2015 at 10:32:41PM +0200, Christophe JAILLET wrote: > > >>>Reference to the 'np' node is dropped before dereferencing the 'sizep' and > > >>>'basep' pointers, which could by then point to junk if the node has been > > >>>freed. > > >>> > > >>>Refactor code to call 'of_node_pup' later. > > >>> > > >>>Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > >>LGTM. > > >Is there anyone able to provide Tested-by for this? > > > > > >Christophe, were you able to reproduce the crash (insmod/rmmod couple > > >of times maybe?) and validate that it was gone after fixing the bug? > > > > Hi, > > no, I never triggered the bug. > > This is just something noticed while looking at potential issues related to > > incorrect use of 'of_node_pup'. > > I only compile tested the patch. > > The fix is so obvious that I see no reason not to include it. Thanks for > the good work. I'm getting $ git am ~/tmp/of-fix.patch Applying: TPM: Avoid reference to potentially freed memory error: patch failed: drivers/char/tpm/tpm_of.c:53 error: drivers/char/tpm/tpm_of.c: patch does not apply Patch failed at 0001 TPM: Avoid reference to potentially freed memory The copy of the patch that failed is found in: /home/jsakkine/projects/tpm2/git/linux-tpmdd/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". I'm applying this against Linus tree (4.3-rc7). /Jarkko ------------------------------------------------------------------------------
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c index 1141456..570f30c 100644 --- a/drivers/char/tpm/tpm_of.c +++ b/drivers/char/tpm/tpm_of.c @@ -53,17 +53,18 @@ int read_log(struct tpm_bios_log *log) goto cleanup_eio; } - of_node_put(np); log->bios_event_log = kmalloc(*sizep, GFP_KERNEL); if (!log->bios_event_log) { pr_err("%s: ERROR - Not enough memory for BIOS measurements\n", __func__); + of_node_put(np); return -ENOMEM; } log->bios_event_log_end = log->bios_event_log + *sizep; memcpy(log->bios_event_log, __va(*basep), *sizep); + of_node_put(np); return 0;
Reference to the 'np' node is dropped before dereferencing the 'sizep' and 'basep' pointers, which could by then point to junk if the node has been freed. Refactor code to call 'of_node_pup' later. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- drivers/char/tpm/tpm_of.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)