fix accidental corrdump

[zjd]

If disturb everyone, I'm sorry.

l->alloc itself address(&l->alloc) in the pool can be reused  rather than l->alloc pointer to wild address, &l->alloc return to pool.
And I try only use large memory with Maxim's way, but it's not coredump. Because coredump is accident, not coredump maybe be reasonable. if l->alloc is not setted NULL after free, the place where use ngx_palloc or ngx_array_push etc, need memzero to avoid wild pointer after use ngx_reset_pool.



At the end, sorry again if disturb everyone.














At 2022-09-30 07:21:29, "Maxim Dounin" <mdounin at mdounin.ru> wrote:
>Hello!
>
>On Thu, Sep 29, 2022 at 04:37:24PM -0400, Frank Swasey wrote:
>
>> This is getting quite tiresome.  You are both stuck in your point of view
>> and refusing to hear what the other one is saying.
>> 
>> Maxim - you keep repeating " l->alloc is not used after free(). "  Clearly,
>> that is not true if setting it to NULL prevents the segfault.  What is true
>> is that NGINX core code does not use it.  As a defensive coding technique,
>> I agree with zjd that setting the pointer you just freed to NULL to
>> indicate to any other code that is checking it is the proper action.  The
>> only other thing that zjd can do is to set the pointer to NULL in their own
>> code after calling the reset function if you are adamant that such
>> defensive measures cannot be put into the NGINX core code.  Any future
>> programmers that write modules like zjd has done that test a pointer for
>> being NULL and use it if it has a non-NULL value, will trip over the same
>> problem, and you can have this argument all over again.
>
>The particular code is internal to nginx core, and the l->alloc is 
>never used after free: this is something which can be easily 
>seen within the ngx_reset_pool() function, which is about 20 
>lines by itself.  That is, setting l->alloc to NULL is not a 
>defensive coding technique by any means, and that's what I've 
>tried to explain.
>
>If setting l->alloc to NULL prevents the segfault, it is 
>accidental, and likely indicate that zjd's module is using 
>uninitialized and/or freed memory somewhere.  Trying to mitigate 
>such bugs by setting arbitrary pointers to NULL is not going to 
>fix these bugs.  Rather, this will make them harder to find and 
>fix.  Instead, actions should be taken to make segfaults due to 
>these bugs more likely, so it would be easier to find and fix 
>them.
>
>I've provided a few pointers on how to get segfaults due to such 
>bugs more likely, hopefully it'll help zjd to find and fix bugs in 
>his code.  In this particular case, I suspect that Address 
>Sanitizer combined with NGX_DEBUG_PALLOC would be enough to 
>immediately identify the bug.
>
>What can be an improvement here is to introduce junk filling in 
>the pool allocator code, for both new allocations and all blocks 
>freed by ngx_reset_pool(), similarly to ngx_slab_junk() as used in 
>the slab allocator.  I think that NGX_DEBUG_PALLOC would be more 
>than enough in this particular case though, as this changes all 
>pool allocations to use system allocator, and therefore makes it 
>possible to configure junk filling and malloc checking on the OS 
>level.
>
>Sorry if this discussion bothers you.  It would be more 
>appropriate in the nginx-devel@ mailing list, but the patch was 
>posted here and it's probably too late to move the discussion 
>anyway.
>
>-- 
>Maxim Dounin
>http://mdounin.ru/
>_______________________________________________
>nginx mailing list -- nginx at nginx.org
>To unsubscribe send an email to nginx-leave at nginx.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx/attachments/20220930/4e4828b6/attachment.htm>