Received-SPF: pass (google.com: domain of linux-kernel+bounces-78073-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 23 Feb 2024 10:46:20 +0100
From: Oleg Nesterov <oleg@redhat.com>
To: Wen Yang <wenyang.linux@foxmail.com>
Cc: Christian Brauner <brauner@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Mike Christie <michael.christie@oracle.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Vincent Whitchurch <vincent.whitchurch@axis.com>,
	Ard Biesheuvel <ardb@kernel.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] kernel/signal.c: explicitly initialize si_code and
 use ksig->info uniformly
Message-ID: <20240223094620.GA8267@redhat.com>
References: <tencent_195B55A5521705954D5EA4873327F8E53D0A@qq.com>
 <20240222190546.GA5993@redhat.com>
 <tencent_3867DFAA296AACA094C9E8F413E6493FF407@qq.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <tencent_3867DFAA296AACA094C9E8F413E6493FF407@qq.com>
User-Agent: Mutt/1.5.24 (2015-08-30)

On 02/23, Wen Yang wrote:
>
> On 2024/2/23 03:05, Oleg Nesterov wrote:
> >On 02/23, wenyang.linux@foxmail.com wrote:
> >>
> >>From: Wen Yang <wenyang.linux@foxmail.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.