mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Petr Skocik <pskocik@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Marco Elver <elver@google.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/1] *** Fix kill(-1,s) returning 0 on 0 kills ***
Date: Fri, 11 Aug 2023 16:25:50 -0500	[thread overview]
Message-ID: <87pm3t2rvl.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <336ae9be-c66c-d87f-61fe-b916e9f04ffc@gmail.com> (Petr Skocik's message of "Thu, 10 Aug 2023 23:30:44 +0200")

Petr Skocik <pskocik@gmail.com> writes:

> Thanks for the detailed analysis, Eric W. Biederman.
>
> All my software really cares about is that I get some indication that a
> kill(-1,s) run from a non-root pid no longer had anything left to kill, 
> which on Linux is currently being masked by a return value of 0 whereas BDSs
> nicely provide an ESRCH. -EPERM would work too (and would still be more useful
> to me than the current behavior), but I will still object to it because I'm
> convinced you're misreading POSIX here and ESRCH, not EPERM, is the error that
> should be returned here.

Thank you for saying any error return is good enough for your
application.  It is definitely a bug that Linux reports success when no
signal has been delivered.

I dug into this a little bit more and found that Illumos and it's
ancestor OpenSolaris can return EPERM, even when sending to all
processes, by reading the Illumos source code.

Reading through the rational of kill it says that it is sometimes
desirable to hide the existence of one process from another so that the
existence of a process will not be an information leak.  To accommodate
that POSIX allows ESRCH instead of EPERM as an error code.

If you want you can read it for yourself here:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/kill.html


To sum up.

The function kill(2) should always report success when it has delivered
a signal and not otherwise.

The Linux version of kill(2) is buggy because it reports success when it
has not delivered a signal.

Different implementations of kill(2) do different things in this
situation and POSIX appears to allow the variation, so there is no
strong argument for any specific behavior (other than returning an
error) from a compatibility standpoint.

From my perspective making the implementation of sending a signal to all
processes (-1) handle errors the same as sending a signal to a process
group (-pgrp) seems like the most sensible way to fix this bug in Linux.

I can see an argument for hiding the existence of processes and
returning ESRCH but if/when we go down that road I would just ask that
we be consistent and update all of the signal sending functions at the
same time.

I will see about writing a commit message and posting a final patch in
just a little bit.

Eric

  reply	other threads:[~2023-08-11 21:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-22 16:12 Petr Skocik
2022-11-22 16:12 ` [PATCH 1/1] Fix kill(-1,s) returning 0 on 0 kills Petr Skocik
2022-11-23 10:30   ` Oleg Nesterov
2022-11-23 11:20     ` Oleg Nesterov
2022-11-23 11:27     ` Petr Skocik
2022-11-23 11:56       ` Oleg Nesterov
2022-11-22 17:15 ` [PATCH 0/1] *** Fix kill(-1,s) returning 0 on 0 kills *** Kees Cook
2022-11-22 23:01   ` Petr Skocik
2023-08-09 12:27   ` Petr Skocik
2023-08-10 16:16     ` Eric W. Biederman
2023-08-10 21:30       ` Petr Skocik
2023-08-11 21:25         ` Eric W. Biederman [this message]
2023-08-11 22:16           ` [PATCH] signal: Fix the error return of kill -1 Eric W. Biederman
2023-08-14 14:06             ` Oleg Nesterov
2023-08-14 15:43               ` Oleg Nesterov
2023-08-15 14:47                 ` David Laight
2023-08-15 15:11                   ` Oleg Nesterov
2023-08-16 20:32                     ` Eric W. Biederman
2023-08-16 21:06                       ` Oleg Nesterov
2023-08-17  2:33                         ` Eric W. Biederman
2023-08-17  4:37                           ` Eric W. Biederman
2023-08-17 15:47                             ` [PATCH] __kill_pgrp_info: simplify the calculation of return value Oleg Nesterov
2023-08-11 23:37           ` [PATCH 0/1] *** Fix kill(-1,s) returning 0 on 0 kills *** Petr Skocik

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=87pm3t2rvl.fsf@email.froward.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=elver@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pskocik@gmail.com \
    --cc=tglx@linutronix.de \
    /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

Powered by JetHome