Re: [PATCH 2/2] Bluetooth: Fix deadlock and crash when SMP pairing times out

[Johan Hedberg <johan.hedberg@xxxxxxxxx>]

Hi Vinicius,

On Wed, Jun 06, 2012, Vinicius Costa Gomes wrote:
> > The l2cap_conn_del function tries to cancel_sync the security timer, but
> > when it's called from the timeout function itself a deadlock occurs.
> > Subsequently the "hcon->l2cap_data = NULL" that's supposed to protect
> > multiple calls to l2cap_conn_del never gets cleared and when the
> > connection finally drops we double free's etc which will crash the
> > kernel.
> 
> I wonder if (inside l2cap_conn_del()) we move "hcon->l2cap_data = NULL"
> up in the function, probably next to the check for "!conn", would be a
> safer alternative.

That was one of the things I tried first as well (and it did remove the
crash) but it doesn't remove the deadlock. The l2cap_conn_del would
still deadlock in cancel_sync(sec_timer) when called from
security_timeout().

Moving the NULL assignment to the top would certainly help decrease the
chance of calling l2cap_conn_del twice but it wouldn't completely remove
this race condition as clearing the variable + testing its value isn't
a single atomic operation.

So I'd still consider this deadlock removal patch by itself as necessary
and valid but additionally the race of calling l2cap_conn_del twice
would need to be fixed with a separate patch, possibly involving some
new or existing lock.

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