linux-kernel - Re: [PATCH 2/2] kernel/signal.c: explicitly initialize si

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240223094620.GA8267@redhat.com>
Date: Fri, 23 Feb 2024 10:46:20 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: Wen Yang <wenyang.linux@...mail.com>
Cc: Christian Brauner <brauner@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Luis Chamberlain <mcgrof@...nel.org>,
	Mike Christie <michael.christie@...cle.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Vincent Whitchurch <vincent.whitchurch@...s.com>,
	Ard Biesheuvel <ardb@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] kernel/signal.c: explicitly initialize si_code and
 use ksig->info uniformly

On 02/23, Wen Yang wrote:
>
> On 2024/2/23 03:05, Oleg Nesterov wrote:
> >On 02/23, wenyang.linux@...mail.com wrote:
> >>
> >>From: Wen Yang <wenyang.linux@...mail.com>
> >>
> >>By explicitly initializing ksig->info.si_code and uniformly using ksig->info,
> >>get_signal() function could be slightly optimized, as folowes:
> >
> >I don't understand. Why do you think it will be optimized? in what sense?
> >
> >>	clear_siginfo(&ksig->info);
> >>	ksig->info.si_signo = signr = SIGKILL;          --> missed si_code
> >
> >because we do not need to set .si_code in this case?
> >
> >>	sigdelset(&current->pending.signal, SIGKILL);
> >>	trace_signal_deliver(SIGKILL, SEND_SIG_NOINFO,  --> unnecessary SEND_SIG_NOINFO
> >
> >Why do you think the usage of SEND_SIG_NOINFO is "unnecessary" or bad?
> >To me this looks good.
> >
>
> Since it is called "SEND_SIG_NOINFO", but here it is neither SEND_SIG
> nor NOINFO.

I don't really understand what does this mean. But I can say that
SEND_SIG_NOINFO is exactly what we should use, this signal has no
info.

In fact, SIGKILL can never have the info, see the sig == SIGKILL
check in __send_signal_locked() but this is offtopic.

> It is get_signal() here, and ksig->info has also been partially
> initialized before calling trace_signal_deliver(). Below "goto fatal",
> do_coredump() also use the initialized ksig->info.

IIRC, do_coredump() paths use only siginfo->si_signo, but this doesn't
matter.

do_coredump() can't be called, sig_kernel_coredump(SIGKILL) is false.

> >and it seems that we can simply kill clear_siginfo(), but this is
> >another story.
>
> This is not right.
>
> ksig->info will be passed to user space through do_coredump(), and the
> clear_siginfo() cannot be killed.

See above.

Oleg.