linux-kernel - Re: [PATCH 2/2] signal: don't allow STOP on PF_IO

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: <m1sg4paj8h.fsf@fess.ebiederm.org>
Date:   Sat, 20 Mar 2021 11:21:50 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Jens Axboe <axboe@...nel.dk>
Cc:     io-uring@...r.kernel.org, torvalds@...ux-foundation.org,
        linux-kernel@...r.kernel.org, oleg@...hat.com,
        Stefan Metzmacher <metze@...ba.org>
Subject: Re: [PATCH 2/2] signal: don't allow STOP on PF_IO_WORKER threads

Jens Axboe <axboe@...nel.dk> writes:

> Just like we don't allow normal signals to IO threads, don't deliver a
> STOP to a task that has PF_IO_WORKER set. The IO threads don't take
> signals in general, and have no means of flushing out a stop either.

At first glance this seems safe.  This is before we count all of the
threads, and it needs to be a non io_uring thread.

Further we can change this decision later if necessary, as this is not
really exposed to userspace.

But please tell me why is it not the right thing to have the io_uring
helper threads stop?  Why is it ok for that process to go on consuming
cpu resources in a non-stoppable way.

Eric


> Reported-by: Stefan Metzmacher <metze@...ba.org>
> Signed-off-by: Jens Axboe <axboe@...nel.dk>
> ---
>  kernel/signal.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 730ecd3d6faf..b113bf647fb4 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2349,6 +2349,10 @@ static bool do_signal_stop(int signr)
>  
>  		t = current;
>  		while_each_thread(current, t) {
> +			/* don't STOP PF_IO_WORKER threads */
> +			if (t->flags & PF_IO_WORKER)
> +				continue;
> +
>  			/*
>  			 * Setting state to TASK_STOPPED for a group
>  			 * stop is always done with the siglock held,