netdev - Re: xfrm_state locking regression...

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <48BE2E63.8000707@iki.fi>
Date:	Wed, 03 Sep 2008 09:27:47 +0300
From:	Timo Teräs <timo.teras@....fi>
To:	David Miller <davem@...emloft.net>
CC:	herbert@...dor.apana.org.au, netdev@...r.kernel.org
Subject: Re: xfrm_state locking regression...

David Miller wrote:
> From: Herbert Xu <herbert@...dor.apana.org.au>
> Date: Wed, 3 Sep 2008 15:50:41 +1000
> 
>> On Wed, Sep 03, 2008 at 08:45:01AM +0300, Timo Teräs wrote:
>>>> ipsec: Move state dump list unlinking into GC
>>>>
>>>> This patch moves the unlinking of x->all (for dumping) into the
>>>> GC and changes the lock to xfrm_cfg_mutex.
>>> Shouldn't this also change the locking in all places where
>>> x->all is used?
>> You're right.  I'd forgotten about the larval states for some
>> reason, probably because I just added them to the list :)
>>
>> So let's keep xfrm_state_lock and just move it.
> 
> How about we un-crap the reference counting?
> 
> Every list an object lives on is an external reference, and that is
> the rule that is violated by this ->all list handling.
> 
> And that's why we need to take a single shared lock twice just to get
> rid of an xfrm_state object now.

Well, it's just another list keeping a reference like ->bydst,
->bysrc and ->byspi. The actual amount of external references is
still correct (the walking code calls _hold() when it returns while
keeping an external pointer).

The difference is that node should not be unlinked from ->all until
all other references are gone. For other lists the unlinking can be
done earlier since they are used only for lookups.

Any good other ways to enumerate to list entries while allowing
to keep a temporary "iterator"? The previous method was crap too.

- Timo
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html