linux-kernel - Re: [PATCH] eventpoll: Fix priority inversion problem

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] [day] [month] [year] [list]

Message-ID: <xhsmh5xhu9dpp.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
Date: Wed, 21 May 2025 17:37:54 +0200
From: Valentin Schneider <vschneid@...hat.com>
To: Nam Cao <namcao@...utronix.de>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner
 <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Sebastian Andrzej Siewior
 <bigeasy@...utronix.de>, John Ogness <john.ogness@...utronix.de>, Clark
 Williams <clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
 linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-rt-devel@...ts.linux.dev, linux-rt-users@...r.kernel.org, Joe Damato
 <jdamato@...tly.com>, Martin Karsten <mkarsten@...terloo.ca>, Jens Axboe
 <axboe@...nel.dk>, Frederic Weisbecker <frederic@...nel.org>
Subject: Re: [PATCH] eventpoll: Fix priority inversion problem

On 21/05/25 16:53, Nam Cao wrote:
> On Wed, May 21, 2025 at 04:25:00PM +0200, Valentin Schneider wrote:
>> > +	llist_for_each_entry_safe(epi, tmp, txlist.first, rdllink) {
>> > +		/*
>> > +		 * We are done iterating. Allow the items we took to be added back to the ready
>> > +		 * list.
>> > +		 */
>> > +		xchg(&epi->link_locked, false);
>> > +
>> > +		/*
>> > +		 * In the loop above, we may mark some items ready, and they should be added back.
>> > +		 *
>> > +		 * Additionally, someone else may also attempt to add the item to the ready list,
>> > +		 * but got blocked by us. Add those blocked items now.
>> > +		 */
>> > +		if (smp_load_acquire(&epi->ready)) {
>> >                       ep_pm_stay_awake(epi);
>> > +			epitem_ready(epi);
>> >               }
>>
>> Isn't this missing a:
>>
>>                 list_del_init(&epi->rdllink);
>>
>> AFAICT we're always going to overwrite that link when next marking the item
>> as ready, but I'd say it's best to exit this with a clean state. That would
>> have to be before the clearing of link_locked so it doesn't race with a
>> concurrent epitem_ready() call.
>
> To confirm I understand you: there is no functional problem, and your
> comment is more of a "just to be safe"?
>

Yup, even if they're not accessed it's good to not have stray references to
stack addresses.

> Thanks so much for the review,
> Nam