Hi Dean,
* Dean Jenkins <djenkins@xxxxxxxxxx> [2012-08-11 19:47:09 +0100]:
> rfcomm_session_timeout() protects the scenario of the remote
> Bluetooth device failing to send a DISC on the rfcomm control
> channel after the last data DLC channel has been closed.
>
> There is a race condition between the timer expiring causing
> rfcomm_session_timeout() to run and the rfcomm session being
> deleted. If the rfcomm session is deleted then
> rfcomm_session_timeout() would use a freed rfcomm session
> pointer resulting in a potential kernel crash or memory corruption.
> Note the timer is cleared before the rfcomm session is deleted
> by del_timer() so the circumstances for a failure to occur are
> as follows:
>
> rfcomm_session_timeout() needs to be executing before the
> del_timer() is called to clear the timer but
> rfcomm_session_timeout() needs to be delayed from using the
> rfcomm session pointer until after the session has been deleted.
> Therefore, there is a very small window of opportunity for failure.
>
> The solution is to use del_timer_sync() instead of del_timer()
> as this ensures that rfcomm_session_timeout() is not running
> when del_timer_sync() returns. This means that it is not
> possible for rfcomm_session_timeout() to run after the rfcomm
> session has been deleted.
>
> Signed-off-by: Dean Jenkins <djenkins@xxxxxxxxxx>
> ---
> net/bluetooth/rfcomm/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 24d4d3c..c7921fd 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -264,8 +264,8 @@ static void rfcomm_session_clear_timer(struct rfcomm_session *s)
> {
> BT_DBG("session %p state %ld", s, s->state);
>
> - if (timer_pending(&s->timer))
> - del_timer(&s->timer);
> + /* ensure rfcomm_session_timeout() is not running past this point */
> + del_timer_sync(&s->timer);
I'm not happy of the idea of let the stack broken between patches 2 and 3
(this one). As you said if we use del_timer_sync() we don't need rfcnt here,
can you add this as a first patch, maybe? and after this continue to remove
the rest of the refcount code?
Gustavo
--
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
