Re: xfrm_state locking regression...
From: David Miller <davem@davemloft.net>
Date: 2008-09-03 06:47:30
From: Timo Teräs <redacted> Date: Wed, 03 Sep 2008 09:45:48 +0300
David Miller wrote:quoted
Once there are no list references, there cannot be any other references. So in fact it seems to me that unlinking when the xfrm_state is removed from those other lists makes perfect sense. If __xfrm_state_delete sets the state to DEAD, and you skip xfrm_state objects marked DEAD, why does the ->all list reference have to survive past __xfrm_state_delete()? It seems the perfect place to do the ->all removal.1. xfrm_state_walk() called, it returns but holds an entry since the walking was interrupted temporarily (e.g. full netlink buffer). 2. xfrm_state_delete() called to the entry that xfrm_state_walk() is keeping a pointer to and it is unlinked. 3. xfrm_state_walk() called again, it tries to resume list walking but whoops, the entry was unlinked and kaboom.
Get creative, use a key of some sort to continue the walk, that's what other netlink'ish subsystems use.
Yes, but the dumping code produced crap. It could dump same entry multiple times, miss entries and was dog slow. With it there was no possibility to keep userland in sync with kernel SPD/SAD because entries were lost.
I'd rather see an entry twice in a dump than have my IPSEC gateway lockup, or run slower because we take a lock twice as often as necessary during object destruction.