public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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
Date: Fri, 23 Feb 2024 10:46:20 +0100	[thread overview]
Message-ID: <20240223094620.GA8267@redhat.com> (raw)
In-Reply-To: <tencent_3867DFAA296AACA094C9E8F413E6493FF407@qq.com>

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.


      reply	other threads:[~2024-02-23  9:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-22 16:04 [PATCH 2/2] kernel/signal.c: explicitly initialize si_code and use ksig->info uniformly wenyang.linux
2024-02-22 19:05 ` Oleg Nesterov
2024-02-23  5:16   ` Wen Yang
2024-02-23  9:46     ` Oleg Nesterov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240223094620.GA8267@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ardb@kernel.org \
    --cc=bigeasy@linutronix.de \
    --cc=brauner@kernel.org \
    --cc=dvyukov@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=michael.christie@oracle.com \
    --cc=tglx@linutronix.de \
    --cc=vincent.whitchurch@axis.com \
    --cc=wenyang.linux@foxmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox