Re: [PATCH 1/2 v2] Bluetooth: Fix system crash caused by del_timer()

["Gustavo F. Padovan" <padovan@profusion.mobi>]

Hi Haijun,

* haijun liu <liuhaijun.er@gmail.com> [2010-10-26 09:32:19 +0800]:

> Hi Gustavo,
> 
> >> >> During test session with another vendor's bt stack, found that in
> >> >> l2cap_chan_del() using del_timer() caused l2cap_monitor_timeout()
> >> >> be called after the sock was freed, so it raised a system crash.
> >> >> So I just replaced del_timer() with del_timer_sync() to solve it.
> >> >
> >> > NAK on this. If you read the del_timer_sync() documentation you can
> >> > see that you can't call del_timer_sync() on interrupt context. The
> >> > possible solution here is to check in the beginning of
> >> > l2cap_monitor_timeout() if your sock is still valid.
> >> >
> >>
> >> You are right, I only considered close() interface, so missed the interrupt
> >> context.
> >>
> >> It's very difficult to check sock valid or not in timeout procedure, since it's
> >> an interrupt context, and only can get context from parameter pre-stored,
> >> except global variables.
> >
> > I think you can check for sk == null there.
> >
> 
> It's a pre-stored parameter, it will not change by itself.

I looked a bit into this and a good solution seems to be to hold a
reference to the sock when we call a mod_timer() and then put the
reference when we call del_timer() and the timer is inactive or when
l2cap_monitor_timeout(). Look net/sctp/ for examples.

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi