Subject: Re: [PATCH][ROSE][AX25] af_ax25: possible circular locking

From: Jarek Poplawski <[email-masked]>
Date: Sun, 30 Dec 2007 15:13:23 +0100

On Sat, Dec 29, 2007 at 07:14:43PM -0800, David Miller wrote:
...
> You can't just drop this linked list lock and expect it to stay
> consistent like that.
> 
> Once you drop it, any thread of control can get in there and delete
> entries from the list.
> 
> Since we know it can happen, using a WARN_ON_ONCE(1) is not
> appropriate.

The problem is 'we' don't know if it can happen... In the first
message with this patch I've tried to get this information, and
now it seems you are the only one with this knowledge, but of
course this is more than enough for me to agree with your decision
to dump this patch.

> [...]  And if it triggers it will do the wrong thing, because
> by branching back to "again" we can call ax25_disconnect() multiple
> times on the same entry which isn't right.

This is an equivalent of list_for_each_entry_safe(), and if such
WARN_ON is ever reported this would confirm the solution is wrong.
But, it seems Bernard's case was too simple to trigger this bug.
Alas it was complex enough to trigger this other bug...

"again" loop should skip this entry next time: s->ax25_dev should
be NULL, so not equal to ax25dev. (But of course there could be
this unknown to me place, which changes this back in the meantime.)

> You'll thus need to resolve this locking conflict more properly.
> I know it's hard, but your current fix is worse because it adds
> a new known bug.

Sorry, it seems this will've to wait until Ralf finds some time,
because I really can't give this more time, considering I never
used nor have any plans to use this code, and this bug could
suggest there is not so many interested in this, besides Bernard,
either.

Regards,
Jarek P.