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

[David Miller <davem@davemloft.net>]

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Fri, 28 Dec 2007 22:48:57 +0100

> lockdep is worried about the different order here:
> 
> #1 (rose_neigh_list_lock){-+..}:
> #3 (ax25_list_lock){-+..}:
> 
> #0 (linkfail_lock){-+..}:
> #1 (rose_neigh_list_lock){-+..}:
> 
> #3 (ax25_list_lock){-+..}:
> #0 (linkfail_lock){-+..}:
> 
> So, ax25_list_lock could be taken before and after linkfail_lock. 
> I don't know if this three-thread clutch is very probable (or
> possible at all), but it seems another bug reported by Bernard
> ("[...] system impossible to reboot with linux-2.6.24-rc5")
> could have similar source - namely ax25_list_lock held by
> ax25_kill_by_device() during ax25_disconnect(). It looks like the
> only place which calls ax25_disconnect() this way, so I guess, it
> isn't necessary.
> 
> This patch is breaking the lock for ax25_disconnect(), with some
> failsafe and debugging added to detect unforeseen problems.
> 
> 
> Reported-and-tested-by: Bernard Pidoux <f6bvp@free.fr>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

I can't apply this fix, sorry.

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.  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.

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.