* 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.