Commit 5e0e02f0 authored by Jens Axboe's avatar Jens Axboe Committed by Thomas Gleixner
Browse files

futex: Pass in task to futex_queue() 



futex_queue() -> __futex_queue() uses 'current' as the task to store in
the struct futex_q->task field. This is fine for synchronous usage of
the futex infrastructure, but it's not always correct when used by
io_uring where the task doing the initial futex_queue() might not be
available later on. This doesn't lead to any issues currently, as the
io_uring side doesn't support PI futexes, but it does leave a
potentially dangling pointer which is never a good idea.

Have futex_queue() take a task_struct argument, and have the regular
callers pass in 'current' for that. Meanwhile io_uring can just pass in
NULL, as the task should never be used off that path. In theory
req->tctx->task could be used here, but there's no point populating it
with a task field that will never be used anyway.

Reported-by: default avatarJann Horn <jannh@google.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/22484a23-542c-4003-b721-400688a0d055@kernel.dk
parent bc8198dc

io_uring/futex.c

+1 −1
Original line number Diff line number Diff line
@@ -338,7 +338,7 @@ int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags) 
		hlist_add_head(&req->hash_node, &ctx->futex_list);

		io_ring_submit_unlock(ctx, issue_flags);



		futex_queue(&ifd->q, hb);

		futex_queue(&ifd->q, hb, NULL);

		return IOU_ISSUE_SKIP_COMPLETE;

	}

kernel/futex/core.c

+3 −2
Original line number Diff line number Diff line
@@ -532,7 +532,8 @@ void futex_q_unlock(struct futex_hash_bucket *hb) 
	futex_hb_waiters_dec(hb);

}



void __futex_queue(struct futex_q *q, struct futex_hash_bucket *hb)

void __futex_queue(struct futex_q *q, struct futex_hash_bucket *hb,

		   struct task_struct *task)

{

	int prio;
@@ -548,7 +549,7 @@ void __futex_queue(struct futex_q *q, struct futex_hash_bucket *hb) 


	plist_node_init(&q->list, prio);

	plist_add(&q->list, &hb->chain);

	q->task = current;

	q->task = task;

}



/**

kernel/futex/futex.h

+8 −3
Original line number Diff line number Diff line
@@ -285,13 +285,15 @@ static inline int futex_get_value_locked(u32 *dest, u32 __user *from) 
}



extern void __futex_unqueue(struct futex_q *q);

extern void __futex_queue(struct futex_q *q, struct futex_hash_bucket *hb);

extern void __futex_queue(struct futex_q *q, struct futex_hash_bucket *hb,

				struct task_struct *task);

extern int futex_unqueue(struct futex_q *q);



/**

 * futex_queue() - Enqueue the futex_q on the futex_hash_bucket

 * @q:	The futex_q to enqueue

 * @hb:	The destination hash bucket

 * @task: Task queueing this futex

 *

 * The hb->lock must be held by the caller, and is released here. A call to

 * futex_queue() is typically paired with exactly one call to futex_unqueue().  The
@@ -299,11 +301,14 @@ extern int futex_unqueue(struct futex_q *q); 
 * or nothing if the unqueue is done as part of the wake process and the unqueue

 * state is implicit in the state of woken task (see futex_wait_requeue_pi() for

 * an example).

 *

 * Note that @task may be NULL, for async usage of futexes.

 */

static inline void futex_queue(struct futex_q *q, struct futex_hash_bucket *hb)

static inline void futex_queue(struct futex_q *q, struct futex_hash_bucket *hb,

			       struct task_struct *task)

	__releases(&hb->lock)

{

	__futex_queue(q, hb);

	__futex_queue(q, hb, task);

	spin_unlock(&hb->lock);

}

kernel/futex/pi.c

+1 −1
Original line number Diff line number Diff line
@@ -982,7 +982,7 @@ int futex_lock_pi(u32 __user *uaddr, unsigned int flags, ktime_t *time, int tryl 
	/*

	 * Only actually queue now that the atomic ops are done:

	 */

	__futex_queue(&q, hb);

	__futex_queue(&q, hb, current);



	if (trylock) {

		ret = rt_mutex_futex_trylock(&q.pi_state->pi_mutex);

kernel/futex/waitwake.c

+2 −2
Original line number Diff line number Diff line
@@ -350,7 +350,7 @@ void futex_wait_queue(struct futex_hash_bucket *hb, struct futex_q *q, 
	 * access to the hash list and forcing another memory barrier.

	 */

	set_current_state(TASK_INTERRUPTIBLE|TASK_FREEZABLE);

	futex_queue(q, hb);

	futex_queue(q, hb, current);



	/* Arm the timer */

	if (timeout)
@@ -461,7 +461,7 @@ int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *woken) 
			 * next futex. Queue each futex at this moment so hb can

			 * be unlocked.

			 */

			futex_queue(q, hb);

			futex_queue(q, hb, current);

			continue;

		}