Re: [PATCH] Destroy ACLs in the reverse order of creation

On 11/30/2013 05:35 AM, Amos Jeffries wrote:
> On 29/11/2013 1:33 p.m., Alex Rousskov wrote:
>> Hello,
>>
>> The attached patch destroys ACLs in the reverse order of creation to
>> avoid destruction segfaults during reconfiguration. I could reproduce
>> segfaults in v3.3-based code. I saw access to the already destroyed ACL
>> memory in trunk; I suspect trunk did not segfault by luck as the bug
>> appears to be there.
>>
>> Group ACLs created later may use other ACLs created earlier. A group ACL
>> must be deleted first so that its AclDeleter can safely access
>> registration status (and avoid double deletion) of the ACLs it uses.
>> Since ACLs are remembered (in Config.aclList) using a singly-linked
>> list, it is difficult to change their deletion order. Instead, we
>> changed their listing order from FIFO to LIFO.
>>
>> As far as I can tell, the ACL storage order is not important for the
>> rest of the code but please let me know if I missed any cases.

> I think we really should be ref-counting these pointers to make it a bit
> more flexible and ensure that they do not get erased while in use.

Yes, of course. Refcounting would also improve the
AclMatchedName-related code significantly. I cannot volunteer to do the
required legwork at this time, unfortunately.

> +0. I have no objections for this going in now while we consider the above.

Will commit soon unless somebody else finds a reason not to.

Thank you,

Alex.
Received on Sat Nov 30 2013 - 17:00:24 MST