Skip to content

Commit 172bbb8

Browse files
JeffyCNgregkh
authored andcommitted
Bluetooth: hidp: fix possible might sleep error in hidp_session_thread
commit 5da8e47d849d3d37b14129f038782a095b9ad049 upstream. It looks like hidp_session_thread has same pattern as the issue reported in old rfcomm: while (1) { set_current_state(TASK_INTERRUPTIBLE); if (condition) break; // may call might_sleep here schedule(); } __set_current_state(TASK_RUNNING); Which fixed at: dfb2fae Bluetooth: Fix nested sleeps So let's fix it at the same way, also follow the suggestion of: https://lwn.net/Articles/628628/ Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> Tested-by: AL Yu-Chen Cho <acho@suse.com> Tested-by: Rohit Vaswani <rvaswani@nvidia.com> Signed-off-by: Marcel Holtmann <marcel@holtmann.org> Cc: Jiri Slaby <jslaby@suse.cz> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 708d19e commit 172bbb8

1 file changed

Lines changed: 22 additions & 11 deletions

File tree

net/bluetooth/hidp/core.c

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#define VERSION "1.2"
3737

3838
static DECLARE_RWSEM(hidp_session_sem);
39+
static DECLARE_WAIT_QUEUE_HEAD(hidp_session_wq);
3940
static LIST_HEAD(hidp_session_list);
4041

4142
static unsigned char hidp_keycode[256] = {
@@ -1068,12 +1069,12 @@ static int hidp_session_start_sync(struct hidp_session *session)
10681069
* Wake up session thread and notify it to stop. This is asynchronous and
10691070
* returns immediately. Call this whenever a runtime error occurs and you want
10701071
* the session to stop.
1071-
* Note: wake_up_process() performs any necessary memory-barriers for us.
1072+
* Note: wake_up_interruptible() performs any necessary memory-barriers for us.
10721073
*/
10731074
static void hidp_session_terminate(struct hidp_session *session)
10741075
{
10751076
atomic_inc(&session->terminate);
1076-
wake_up_process(session->task);
1077+
wake_up_interruptible(&hidp_session_wq);
10771078
}
10781079

10791080
/*
@@ -1180,20 +1181,20 @@ static void hidp_session_run(struct hidp_session *session)
11801181
struct sock *ctrl_sk = session->ctrl_sock->sk;
11811182
struct sock *intr_sk = session->intr_sock->sk;
11821183
struct sk_buff *skb;
1184+
DEFINE_WAIT_FUNC(wait, woken_wake_function);
11831185

1186+
add_wait_queue(&hidp_session_wq, &wait);
11841187
for (;;) {
11851188
/*
11861189
* This thread can be woken up two ways:
11871190
* - You call hidp_session_terminate() which sets the
11881191
* session->terminate flag and wakes this thread up.
11891192
* - Via modifying the socket state of ctrl/intr_sock. This
11901193
* thread is woken up by ->sk_state_changed().
1191-
*
1192-
* Note: set_current_state() performs any necessary
1193-
* memory-barriers for us.
11941194
*/
1195-
set_current_state(TASK_INTERRUPTIBLE);
11961195

1196+
/* Ensure session->terminate is updated */
1197+
smp_mb__before_atomic();
11971198
if (atomic_read(&session->terminate))
11981199
break;
11991200

@@ -1227,11 +1228,22 @@ static void hidp_session_run(struct hidp_session *session)
12271228
hidp_process_transmit(session, &session->ctrl_transmit,
12281229
session->ctrl_sock);
12291230

1230-
schedule();
1231+
wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
12311232
}
1233+
remove_wait_queue(&hidp_session_wq, &wait);
12321234

12331235
atomic_inc(&session->terminate);
1234-
set_current_state(TASK_RUNNING);
1236+
1237+
/* Ensure session->terminate is updated */
1238+
smp_mb__after_atomic();
1239+
}
1240+
1241+
static int hidp_session_wake_function(wait_queue_t *wait,
1242+
unsigned int mode,
1243+
int sync, void *key)
1244+
{
1245+
wake_up_interruptible(&hidp_session_wq);
1246+
return false;
12351247
}
12361248

12371249
/*
@@ -1244,7 +1256,8 @@ static void hidp_session_run(struct hidp_session *session)
12441256
static int hidp_session_thread(void *arg)
12451257
{
12461258
struct hidp_session *session = arg;
1247-
wait_queue_t ctrl_wait, intr_wait;
1259+
DEFINE_WAIT_FUNC(ctrl_wait, hidp_session_wake_function);
1260+
DEFINE_WAIT_FUNC(intr_wait, hidp_session_wake_function);
12481261

12491262
BT_DBG("session %p", session);
12501263

@@ -1254,8 +1267,6 @@ static int hidp_session_thread(void *arg)
12541267
set_user_nice(current, -15);
12551268
hidp_set_timer(session);
12561269

1257-
init_waitqueue_entry(&ctrl_wait, current);
1258-
init_waitqueue_entry(&intr_wait, current);
12591270
add_wait_queue(sk_sleep(session->ctrl_sock->sk), &ctrl_wait);
12601271
add_wait_queue(sk_sleep(session->intr_sock->sk), &intr_wait);
12611272
/* This memory barrier is paired with wq_has_sleeper(). See

0 commit comments

Comments
 (0)