* Gustavo F. Padovan <padovan@xxxxxxxxxxxxxx> [2011-06-29 17:24:56 -0300]: > * Ilia, Kolominsky <iliak@xxxxxx> [2011-06-26 09:16:58 +0200]: > > > Hi! > > IMHO the fix isnt good due to possible race condition which > > will destroy session/task objects - either by a call to kthread_stop > > from the timer func or reentry to hidp_del_connection() on > > smp platforms. > > see the intopic comments > > > > > > > -----Original Message----- > > > From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth- > > > owner@xxxxxxxxxxxxxxx] On Behalf Of Peter Hurley > > > Sent: Sunday, June 26, 2011 12:33 AM > > > To: linux-bluetooth > > > Subject: [PATCH] Bluetooth: Fix hidp disconnect deadlock > > > > > > Release reader lock on r/w sem before stopping khidp thread (which > > > needs to > > > claim the writer lock on sem before unlinking the session). > > > > > > NB: kthread_stop waits for thread completion. > > > --- > > > net/bluetooth/hidp/core.c | 5 ++++- > > > 1 files changed, 4 insertions(+), 1 deletions(-) > > > > > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > > > index c405a95..16d75d7 100644 > > > --- a/net/bluetooth/hidp/core.c > > > +++ b/net/bluetooth/hidp/core.c > > > @@ -1096,6 +1096,7 @@ int hidp_del_connection(struct hidp_conndel_req > > > *req) > > > { > > > struct hidp_session *session; > > > int err = 0; > > > + bool stop_thread = false; > > > > > > BT_DBG(""); > > > > > > @@ -1111,12 +1112,14 @@ int hidp_del_connection(struct hidp_conndel_req > > > *req) > > > skb_queue_purge(&session->ctrl_transmit); > > > skb_queue_purge(&session->intr_transmit); > > > > > > - kthread_stop(session->task); > > > + stop_thread = true; > > > } > > > } else > > > err = -ENOENT; > > > > > > up_read(&hidp_session_sem); > > > + if (stop_thread) > > > > Timer fires here - session is destroyed. > > > > > + kthread_stop(session->task); > > > return err; > > > } > > > > > > -- > > > 1.7.4.1 > > > > > > -- > > > 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 > > > > I am working on a better solution which will ensure the async deletion > > of session kthread operates correctly from different exec. contexts > > Are you going to deliver this soon? This needs to be fixed on 3.0. This should fix the timer issue. Please test. Gustavo Author: Gustavo F. Padovan <padovan@xxxxxxxxxxxxxx> Date: Wed Jun 29 17:45:56 2011 -0300 Bluetooth: Delete timer before call kthread_stop() If we keep the timer running we can have a race condition between the timer and hidp_del_connection. By deleting the time before call kthread_stop() we won't have any race condition. Signed-off-by: Gustavo F. Padovan <padovan@xxxxxxxxxxxxxx> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 16d75d7..a86ba69 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -476,7 +476,7 @@ static void hidp_set_timer(struct hidp_session *session) static inline void hidp_del_timer(struct hidp_session *session) { if (session->idle_to > 0) - del_timer(&session->timer); + del_timer_sync(&session->timer); } static void hidp_process_handshake(struct hidp_session *session, @@ -1113,6 +1113,8 @@ int hidp_del_connection(struct hidp_conndel_req *req) skb_queue_purge(&session->intr_transmit); stop_thread = true; + + hidp_del_timer(session); } } else err = -ENOENT; ~ -- 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
