Re: [PATCH] BUG: libiptc chain references bug

[Pablo Neira Ayuso <pablo@netfilter.org>]

Jesper Dangaard Brouer wrote:
> On Sun, 16 Jul 2006, Pablo Neira Ayuso wrote:
>> Jesper Dangaard Brouer wrote:
>>>
>>> Correcting a chain references increment bug in libiptc.
>>>
>>> The bug lies in function iptc_delete_entry() / TC_DELETE_ENTRY.  The
>>> problem is the construction of "r" the rule entry, that is used for
>>> comparison. The problem is that the function iptcc_map_target()
>>> increase the target chains references count.
>>>
>>> The fix is to use function iptcc_delete_rule() to delete the "r" rule
>>> (as it decrement the counter again). To make it work a small NULL
>>> pointer check is also added iptcc_delete_rule().
>>>
>>> Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk>
>>
>>
>> I don't like too much the is-the-rule-in-list checking in 
>> delete_entry, please, could you tell me what you think about the patch 
>> attached? I think it's cleaner. Thanks.
> 
> First of all, (no offence) your patch will not work, as it does not 
> catch all control-flow cases. That is, if no match was found your patch 
> does not decrement the refcount. Hint, look at the places free(r) is 
> called.

You are right, I missed that exit path.

> I have attached a patch, that does catch all cases.  This is achived by 
> adding a else statement to the if statement where iptcc_map_target is 
> called.

I think I prefer this one, let see what Patrick thinks.

> I did consider, your strategy, but the reason I decided not to, was that 
> I though it was cleaner to always call "iptcc_delete_rule" when we want 
> to delete a rule, instead of free'ing it manually.
> 
> Well, I think Patrick should make the decision. As long as we fix the 
> bug, I don't care which patch goes in.
> 
> Cheers,
>   Jesper Brouer
> 
> p.s. Pablo, I was not going to piss in the bush... I was just discussing 
> with Gert Hansen about how dry the plant was... Perhaps the plant would 
> have preferred that I would have taken a leak... See you around ;-)

That bush surely would prefer it these damned hot days of summer 8)

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of 
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris