linux-kernel - Re: futex+io_uring: futex_q::task can maybe be dangling (but is not actually accessed, so it's fine)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <30a6d768-b1b8-4adf-8ff0-9f54edde9605@kernel.dk>
Date: Wed, 15 Jan 2025 08:23:39 -0700
From: Jens Axboe <axboe@...nel.dk>
To: Thomas Gleixner <tglx@...utronix.de>,
 Peter Zijlstra <peterz@...radead.org>
Cc: Jann Horn <jannh@...gle.com>, Ingo Molnar <mingo@...hat.com>,
 Darren Hart <dvhart@...radead.org>, Davidlohr Bueso <dave@...olabs.net>,
 André Almeida <andrealmeid@...lia.com>,
 kernel list <linux-kernel@...r.kernel.org>,
 Pavel Begunkov <asml.silence@...il.com>, io-uring <io-uring@...r.kernel.org>
Subject: Re: futex+io_uring: futex_q::task can maybe be dangling (but is not
 actually accessed, so it's fine)

On 1/15/25 3:20 AM, Thomas Gleixner wrote:
> On Mon, Jan 13 2025 at 15:38, Peter Zijlstra wrote:
>> On Fri, Jan 10, 2025 at 08:33:34PM -0700, Jens Axboe wrote:
>>
>>> @@ -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;
>>>  }
>>>  
>>>  /**
>>
>> The alternative is, I suppose, to move the q->task assignment out to
>> these two callsites instead. Thomas, any opinions?
> 
> That's fine as long as hb->lock is held, but the explicit argument makes
> all of this simpler to understand.
> 
> Though I'm not really a fan of this part:
> 
>> +		__futex_queue(&ifd->q, hb, NULL);
>> +		spin_unlock(&hb->lock);
> 
> Can we please add that @task argument to futex_queue() and keep the
> internals in the futex code instead of pulling more stuff into io_uring?

Sure, was trying to keep the change more minimal, but we can certainly
add it to futex_queue() instead rather than needing to work around it on
the io_uring side.

I'll be happy to send out a patch for that.

-- 
Jens Axboe