linux-kernel - Re: [PATCH 1/1] exit: change the release_task() paths to call flush

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: <20250206162301.GC5209@redhat.com>
Date: Thu, 6 Feb 2025 17:23:02 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Frederic Weisbecker <frederic@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Mateusz Guzik <mjguzik@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] exit: change the release_task() paths to call
 flush_sigqueue() lockless

On 02/06, Thomas Gleixner wrote:
>
> On Wed, Feb 05 2025 at 18:51, Oleg Nesterov wrote:
> > A task can block a signal, accumulate up to RLIMIT_SIGPENDING sigqueues,
> > and exit. In this case __exit_signal()->flush_sigqueue() called with irqs
> > disabled can triger a hard lockup, see
> > https://lore.kernel.org/all/20190322114917.GC28876@redhat.com/
> >
> > Fortunately, after the recent posixtimer changes sys_timer_delete() paths
> > no longer try to clear SIGQUEUE_PREALLOC and/or free tmr->sigq, and after
> > the exiting task passes __exit_signal() lock_task_sighand() can't succeed
> > and pid_task(tmr->it_pid) will return NULL.
> >
> > This means that after __exit_signal(tsk) nobody can play with tsk->pending
> > or (if group_dead) with tsk->signal->shared_pending, so release_task() can
> > safely call flush_sigqueue() after write_unlock_irq(&tasklist_lock).
>
> I can't find a problem with that.

Ah, good.

> > Also, kill clear_tsk_thread_flag(TIF_SIGPENDING), it was never needed.
>
> I'm not entirely sure about that, but it does not hurt to clear it,
> right?

Please see v2 which documents this change in a separate patch.
Again, it is not that it is really bad, just it looks very confusing
to me and I think it can confuse other readers of this code.

> > 	- do_sigaction() can hit the similar problem
>
> Indeed, but that's a tough on to solve.

Yeah... Although I have to admit that yesterday I had a very simple
(and wrong) solution in mind ;)

Thanks!

Oleg.