linux-kernel - Re: [PATCH 2/2] io_uring: fix uring_flush in exit

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

Message-ID: <4a938355-94f4-4b90-f10e-270df7c2a0d9@gmail.com>
Date:   Sun, 17 Jan 2021 02:40:22 +0000
From:   Pavel Begunkov <asml.silence@...il.com>
To:     Hillf Danton <hdanton@...a.com>
Cc:     Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org,
        io-uring@...r.kernel.org,
        syzbot+a32b546d58dde07875a1@...kaller.appspotmail.com
Subject: Re: [PATCH 2/2] io_uring: fix uring_flush in exit_files() warning

On 17/01/2021 02:31, Hillf Danton wrote:
> On Sat, 16 Jan 2021 05:32:30 +0000 Pavel Begunkov wrote:
>>
>> @@ -9126,7 +9126,10 @@ static int io_uring_flush(struct file *file, void *data)
>>  
>>  	if (ctx->flags & IORING_SETUP_SQPOLL) {
>>  		/* there is only one file note, which is owned by sqo_task */
>> -		WARN_ON_ONCE((ctx->sqo_task == current) ==
>> +		WARN_ON_ONCE(ctx->sqo_task != current &&
>> +			     xa_load(&tctx->xa, (unsigned long)file));
>> +		/* sqo_dead check is for when this happens after cancellation */
>> +		WARN_ON_ONCE(ctx->sqo_task == current && !ctx->sqo_dead &&
>>  			     !xa_load(&tctx->xa, (unsigned long)file));
>>  
>>  		io_disable_sqo_submit(ctx);
> 
> The added sqo_dead flag can not only quiesce a warning but save a
> disabling dryrun.

Don't think I get the sentence. Do you see any issue?

sqo_dead has a practical meaning, it prevents SQPOLL task from poking
into the creator task when it's racy. But yes, also in some cases makes
draining and killing rings nicer. 

-- 
Pavel Begunkov