Subject: Re: xfrm_state locking regression...

From: Timo Teräs <[email-masked]>
Date: Wed, 24 Sep 2008 09:04:30 +0300

Herbert Xu wrote:
> On Wed, Sep 24, 2008 at 08:46:09AM +0300, Timo Teräs wrote:
>> Well, it's also because in the dump routine the entries
>> are cast to xfrm_state, even if it was a walker entry. This
>> is just wrong, though it probably works since only the specific
>> entries are used.
> 
> No it works because all walker entries set the state to DEAD
> so they're skipped by everybody else.

Yes, I know. I was pointing the fact that the walker function
iterates using struct xfrm_state. So temporarily when it is
iterating through the walker entry, we get strct xfrm_state
pointer which points to some place before the struct xfrm_state_walk.
Now since the km.state is checked first, those are skipped.

I find it very confusing to let the code say "iterate through
list of struct xfrm_state" when it is not such a list. It is a
list of struct xfrm_state or struct xfrm_state_walk. So I'd use
some intermediate struct to so the code can say e.g "iterate
through list of struct xfrm_state_dump_entry" or whatever.

Or at least add a comment to the dumping function to say that
we have struct xfrm_state, but in matter of fact it can be
also struct xfrm_state_walk pointer with displacement, so we
better check km.state first.

Cheers,
  Timo