linux-kernel - Re: [PATCH] signal: Allow tasks to cache one sigqueue struct (again).

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: <Y4cpRRMXiA6RHsXL@linutronix.de>
Date:   Wed, 30 Nov 2022 10:58:29 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Ben Segall <bsegall@...gle.com>,
        Christian Brauner <christian.brauner@...ntu.com>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Ingo Molnar <mingo@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Mel Gorman <mgorman@...e.de>, Oleg Nesterov <oleg@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Valentin Schneider <vschneid@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>
Subject: Re: [PATCH] signal: Allow tasks to cache one sigqueue struct (again).

On 2022-11-29 12:06:22 [-0800], Linus Torvalds wrote:
> In other words, this is *really* meant as not a good patch, but as a
> "can we please go in this direction instead".
> 
> And yes, maybe some other "free this siginfo" paths should *also* do
> that "try to add it to the cache, and if so, you're done" after doing
> the accounting. So this is probably all mis-guided, but I ended up
> wanting to just see how an alternative approach would look, and this
> feels a lot safer to me.

That one case which was not covered by the sighand lock was clearly an
oversight. It should have been under the sighand. The commit description
said so.

> We already have that SIGQUEUE_PREALLOC case, why not use some of the
> same logic for the cached entry. Sure, it bypasses the rlimit, but
> what else is new?

You need set sigqueue_cache to NULL in copy_process().

The SIGQUEUE_PREALLOC signals work slightly different. It is allocated
once at timer_create(2) time and can send a signal multiple times before
finally removed by timer_delete(2). So the signal delivery is, in a way,
already cached. The caching here for timer_create(2) does not work as
__send_signal_locked() (where you grab the cached struct) isn't used by
that code using SIGQUEUE_PREALLOC.
Instead it allocates its sigqueue via sigqueue_alloc() while not holding
the sighand lock.
So after a timer_delete(2), the next signal it receives (which is not
from a posix timer/ SIGQUEUE_PREALLOC) would be from the cache.

>               Linus

Sebastian