* Double free vulnerability in do_rename_gpt_parts
@ 2020-01-17 12:03 Jordy
2020-01-17 15:29 ` Simon Goldschmidt
0 siblings, 1 reply; 5+ messages in thread
From: Jordy @ 2020-01-17 12:03 UTC (permalink / raw)
To: u-boot
Hello U-Boot lists!
I think I found a double free bug in U-Boot, in /cmp/gpt.c in the function do_rename_gpt_parts().
On line 702 the partition_list is being free'd if ret is smaller than 0.
If the return value is not -ENOMEM it will go to the out: label and free the partition_list again.
Double freeing may result in a write-what-where condition, allowing an attacker to execute arbitrary code.
My advice would be to not free the parition_list on line 702 as nothing is being done with it there afterwards anyway and leave your clean_up in the out: label :)
Kind Regards,
Jordy Zomer
^ permalink raw reply [flat|nested] 5+ messages in thread
* Double free vulnerability in do_rename_gpt_parts
2020-01-17 12:03 Double free vulnerability in do_rename_gpt_parts Jordy
@ 2020-01-17 15:29 ` Simon Goldschmidt
2020-01-17 16:31 ` Tom Rini
0 siblings, 1 reply; 5+ messages in thread
From: Simon Goldschmidt @ 2020-01-17 15:29 UTC (permalink / raw)
To: u-boot
+ Some contributors of this file
On Fri, Jan 17, 2020 at 1:03 PM Jordy <jordy@simplyhacker.com> wrote:
>
> Hello U-Boot lists!
>
> I think I found a double free bug in U-Boot, in /cmp/gpt.c in the function do_rename_gpt_parts().
>
> On line 702 the partition_list is being free'd if ret is smaller than 0.
> If the return value is not -ENOMEM it will go to the out: label and free the partition_list again.
Reading the code, I can confirm that. Funny enough, the code in question was
introduced by commit 18030d04 ("GPT: fix memory leaks identified by Coverity").
Although I think Coverity should have detected the resulting double-free...
However, I'm not sure of the fix: the code just continues for -ENOMEM and then
goes on using partitions_list at line 757...
Regards,
Simon
>
> Double freeing may result in a write-what-where condition, allowing an attacker to execute arbitrary code.
>
> My advice would be to not free the parition_list on line 702 as nothing is being done with it there afterwards anyway and leave your clean_up in the out: label :)
>
> Kind Regards,
>
> Jordy Zomer
^ permalink raw reply [flat|nested] 5+ messages in thread
* Double free vulnerability in do_rename_gpt_parts
2020-01-17 15:29 ` Simon Goldschmidt
@ 2020-01-17 16:31 ` Tom Rini
2020-01-17 16:42 ` Simon Goldschmidt
2020-01-17 17:23 ` Jordy
0 siblings, 2 replies; 5+ messages in thread
From: Tom Rini @ 2020-01-17 16:31 UTC (permalink / raw)
To: u-boot
On Fri, Jan 17, 2020 at 04:29:52PM +0100, Simon Goldschmidt wrote:
> + Some contributors of this file
>
> On Fri, Jan 17, 2020 at 1:03 PM Jordy <jordy@simplyhacker.com> wrote:
> >
> > Hello U-Boot lists!
> >
> > I think I found a double free bug in U-Boot, in /cmp/gpt.c in the function do_rename_gpt_parts().
> >
> > On line 702 the partition_list is being free'd if ret is smaller than 0.
> > If the return value is not -ENOMEM it will go to the out: label and free the partition_list again.
>
> Reading the code, I can confirm that. Funny enough, the code in question was
> introduced by commit 18030d04 ("GPT: fix memory leaks identified by Coverity").
> Although I think Coverity should have detected the resulting double-free...
>
> However, I'm not sure of the fix: the code just continues for -ENOMEM and then
> goes on using partitions_list at line 757...
So, Coverity later did complain about that change (but not immediately,
funny enough). I posted http://patchwork.ozlabs.org/patch/1192036/ and
was hoping for a review on it as it's complex enough I'd like to avoid
adding a 3rd round of issues there. Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200117/a3bd537a/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Double free vulnerability in do_rename_gpt_parts
2020-01-17 16:31 ` Tom Rini
@ 2020-01-17 16:42 ` Simon Goldschmidt
2020-01-17 17:23 ` Jordy
1 sibling, 0 replies; 5+ messages in thread
From: Simon Goldschmidt @ 2020-01-17 16:42 UTC (permalink / raw)
To: u-boot
Am 17.01.2020 um 17:31 schrieb Tom Rini:
> On Fri, Jan 17, 2020 at 04:29:52PM +0100, Simon Goldschmidt wrote:
>> + Some contributors of this file
>>
>> On Fri, Jan 17, 2020 at 1:03 PM Jordy <jordy@simplyhacker.com> wrote:
>>>
>>> Hello U-Boot lists!
>>>
>>> I think I found a double free bug in U-Boot, in /cmp/gpt.c in the function do_rename_gpt_parts().
>>>
>>> On line 702 the partition_list is being free'd if ret is smaller than 0.
>>> If the return value is not -ENOMEM it will go to the out: label and free the partition_list again.
>>
>> Reading the code, I can confirm that. Funny enough, the code in question was
>> introduced by commit 18030d04 ("GPT: fix memory leaks identified by Coverity").
>> Although I think Coverity should have detected the resulting double-free...
>>
>> However, I'm not sure of the fix: the code just continues for -ENOMEM and then
>> goes on using partitions_list at line 757...
>
> So, Coverity later did complain about that change (but not immediately,
> funny enough). I posted http://patchwork.ozlabs.org/patch/1192036/ and
> was hoping for a review on it as it's complex enough I'd like to avoid
> adding a 3rd round of issues there. Thanks!
>
Ah, yes. I've just responded there.
Regards,
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
* Double free vulnerability in do_rename_gpt_parts
2020-01-17 16:31 ` Tom Rini
2020-01-17 16:42 ` Simon Goldschmidt
@ 2020-01-17 17:23 ` Jordy
1 sibling, 0 replies; 5+ messages in thread
From: Jordy @ 2020-01-17 17:23 UTC (permalink / raw)
To: u-boot
Hey Tom,
The patch looks good to me, I do agree with simon that you'd have to initialize the pointers to 0, because uninitialized pointers could contain garbage, unless it's a global variable (those are automagically initialized to 0).
Btw I really appreciate the fast and friendly responses, I've seen that different at other software packages thumbs-up for that!
Best Regards,
Jordy
> On January 17, 2020 11:31 AM Tom Rini <trini@konsulko.com> wrote:
>
>
> On Fri, Jan 17, 2020 at 04:29:52PM +0100, Simon Goldschmidt wrote:
> > + Some contributors of this file
> >
> > On Fri, Jan 17, 2020 at 1:03 PM Jordy <jordy@simplyhacker.com> wrote:
> > >
> > > Hello U-Boot lists!
> > >
> > > I think I found a double free bug in U-Boot, in /cmp/gpt.c in the function do_rename_gpt_parts().
> > >
> > > On line 702 the partition_list is being free'd if ret is smaller than 0.
> > > If the return value is not -ENOMEM it will go to the out: label and free the partition_list again.
> >
> > Reading the code, I can confirm that. Funny enough, the code in question was
> > introduced by commit 18030d04 ("GPT: fix memory leaks identified by Coverity").
> > Although I think Coverity should have detected the resulting double-free...
> >
> > However, I'm not sure of the fix: the code just continues for -ENOMEM and then
> > goes on using partitions_list at line 757...
>
> So, Coverity later did complain about that change (but not immediately,
> funny enough). I posted http://patchwork.ozlabs.org/patch/1192036/ and
> was hoping for a review on it as it's complex enough I'd like to avoid
> adding a 3rd round of issues there. Thanks!
>
> --
> Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-17 17:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 12:03 Double free vulnerability in do_rename_gpt_parts Jordy
2020-01-17 15:29 ` Simon Goldschmidt
2020-01-17 16:31 ` Tom Rini
2020-01-17 16:42 ` Simon Goldschmidt
2020-01-17 17:23 ` Jordy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.