* the issue exists ... [in] the newest version To show that, you'd need to show get_buffer being reentered during the xrealloc call. That code is still as it was:
https://github.com/mirror/make/blob/master/src/output.c#L395 static char * get_buffer (size_t need) { if (need > fmtbuf.size) { fmtbuf.size += need * 2; fmtbuf.buffer = xrealloc (fmtbuf.buffer, fmtbuf.size); But, if I understand it correctly, the contention is that the code that was calling get_buffer when malloc fails during xrealloc was removed in that old patch I sent: https://git.savannah.gnu.org/cgit/make.git/commit/?id=68be4f74fce91b76e5915449268d6b5eb687aab9 It's obfuscated by some macro trickery but I think OUT_OF_MEM used to call fatal, which calls get_buffer. Its replacement, out_of_memory, just writes an error message, without any localization, rather puzzlingly to stdout rather than stderr, then exits. The function called fatal is no longer involved. You could argue that the above code has a latent risk. You could argue that it might be less liable to cause trouble under maintenance if an additional local variable were used to hold the new size until xrealloc has returned. Isn’t that gilding a lily? If it ends up looking like something that could be simplified, then a maintainer would benefit from a code-comment warning them that we were once bitten by reentry here, at which point the added complexity might cause more trouble than it's likely to save us from. ________________________________ From: bug-make-bounces+martin.dorey=hds....@gnu.org <bug-make-bounces+martin.dorey=hds....@gnu.org> on behalf of Haoxin Tu <haoxi...@gmail.com> Sent: Saturday, January 20, 2024 20:03 To: Martin Dorey <martin.do...@hitachivantara.com> Cc: psm...@gnu.org <psm...@gnu.org>; bug-make@gnu.org <bug-make@gnu.org> Subject: Re: [bug #64551] Possible null pointer dereference on the function get_buffer ***** EXTERNAL EMAIL ***** Hi all, May I know if you are planning to propose a fix for it? I checked the implementations of other `make` versions a bit further, and as far as I can tell, the issue exists from the older make-4.0.90<https://alpha.gnu.org/gnu/make/make-4.0.90.tar.gz> (2014-9-30) to the newest version of make (make-4.4.0.91<https://alpha.gnu.org/gnu/make/make-4.4.0.91.tar.gz>). Please kindly let me know if I can help any further. Thank you all so much for your time and clarifications again! Best regards, Haoxin Haoxin Tu <haoxi...@gmail.com<mailto:haoxi...@gmail.com>> 于2024年1月21日周日 00:10写道： Hi Paul and Martin, ->We know that OUT_OF_MEM() never returns. So there's no way this function can return 0. I totally agree with you all that the function `xrealloc` itself can never return 0. ->It will, as Martin suggests, recurse infinitely (one assumes) because fatal() calls xrealloc() again, and malloc() will return 0, so it will call fatal(), which calls xrealloc() again, and malloc will return 0, so it will call fatal(), etc. etc.--this is what I meant by my imprecise comment "infinite loop" I should have said "infinite recursion". Yeah, I also agree with your explanation here for the normal scenario, but the potential error I reported here is the situation: fatal() calls xrealloc() again, and malloc() will return 0, so it will call fatal(), which DOES NOT calls xrealloc() again but directly dereferences the pointer, then leading to the error. ->There's a nice catch there - where, in that recursive failure, the writing of that terminator overflows a buffer that wasn't actually reallocated yet. Yeah, hope I made my points clear now. Thank you all so much again. Best regards, Haoxin Martin Dorey <martin.do...@hitachivantara.com<mailto:martin.do...@hitachivantara.com>> 于2024年1月21日周日 00:02写道： * the link you provided seems old Indeed, but that's because, as Paul noted, you're testing code that's even older. * `fmtbuf.size`, defined in .<https://github.com/mirror/make/blob/4.2/output.c#L593>.., was increased after the previous innovation of `get_buffer`), and then the execution of `fmtbuf.buffer[need-1] = '\0';` There's a nice catch there - where, in that recursive failure, the writing of that terminator overflows a buffer that wasn't actually reallocated yet. ________________________________ From: Paul Smith <psm...@gnu.org<mailto:psm...@gnu.org>> Sent: Saturday, January 20, 2024 07:51 To: Haoxin Tu <haoxi...@gmail.com<mailto:haoxi...@gmail.com>>; Martin Dorey <martin.do...@hitachivantara.com<mailto:martin.do...@hitachivantara.com>> Cc: bug-make@gnu.org<mailto:bug-make@gnu.org> <bug-make@gnu.org<mailto:bug-make@gnu.org>> Subject: Re: [bug #64551] Possible null pointer dereference on the function get_buffer ***** EXTERNAL EMAIL ***** On Sat, 2024-01-20 at 23:37 +0800, Haoxin Tu wrote: > But I don't understand why the second invocation of `xrealloc` can > not return zero, I apologize for any imprecise information I provided > in the previous emails. Because of what I said in my original reply: > the entire point of xrealloc is that it never returns 0. Look at the implementation of xrealloc(): void *result = malloc (size ? size : 1); if (result == 0) OUT_OF_MEM(); return result; We know that OUT_OF_MEM() never returns. So there's no way this function can return 0. It will, as Martin suggests, recurse infinitely (one assumes) because fatal() calls xrealloc() again, and malloc() will return 0, so it will call fatal(), which calls xrealloc() again, and malloc will return 0, so it will call fatal(), etc. etc.--this is what I meant by my imprecise comment "infinite loop" I should have said "infinite recursion". As a reminder this is moot: this code has been rewritten and even the infinite recursion problem was removed from the code back in 2017. -- Paul D. Smith <psm...@gnu.org<mailto:psm...@gnu.org>> Find some GNU make tips at: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.gnu.org%2F&data=05%7C02%7CMartin.Dorey%40hitachivantara.com%7C64b0e96f7b7a4d9cf0f308dc19cfa672%7C18791e1761594f52a8d4de814ca8284a%7C0%7C0%7C638413626848068646%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=2CgTubth7nezTPOU76hCwauMdV%2B8cz%2F9415iVpL%2FVe0%3D&reserved=0<https://www.gnu.org/> https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmake.mad-scientist.net%2F&data=05%7C02%7CMartin.Dorey%40hitachivantara.com%7C64b0e96f7b7a4d9cf0f308dc19cfa672%7C18791e1761594f52a8d4de814ca8284a%7C0%7C0%7C638413626848074885%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=0jGEsG8MYYh26rgSn2n8dPl1eydBrKFvkzD0UqnvbuY%3D&reserved=0<http://make.mad-scientist.net/> "Please remain calm...I may be mad, but I am a professional." --Mad Scientist
