Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Eric W. Biederman" <ebiederm@xmission.com>
To:     Florian Mayer <fmayer@google.com>
Cc:     Jonathan Corbet <corbet@lwn.net>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-doc@vger.kernel.org, Oleg Nesterov <oleg@redhat.com>,
        Christian Brauner <brauner@kernel.org>,
        Evgenii Stepanov <eugenis@google.com>,
        Peter Collingbourne <pcc@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Kees Cook <keescook@chromium.org>, linux-api@vger.kernel.org
References: <20220909180617.374238-1-fmayer@google.com>
        <87v8pw8bkx.fsf@email.froward.int.ebiederm.org>
        <CAJAyTCCcecgqeMfs9W8=U4wi-6O+DaRktUsyJuStYy-JgKQCdg@mail.gmail.com>
Date:   Sun, 18 Sep 2022 17:04:48 -0500
In-Reply-To: <CAJAyTCCcecgqeMfs9W8=U4wi-6O+DaRktUsyJuStYy-JgKQCdg@mail.gmail.com>
        (Florian Mayer's message of "Fri, 9 Sep 2022 16:05:34 -0700")
Message-ID: <875yhk730f.fsf@email.froward.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH RESEND] Add sicode to /proc/<PID>/stat.
Precedence: bulk

Florian Mayer <fmayer@google.com> writes:

> On Fri, 9 Sept 2022 at 14:47, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> Added linux-api because you are changing the api.
>
> Thanks.
>
>> Several things.  First you are messing with /proc/<pid>/stat which is
>> heavily used.  You do add the value to the end of the list which is
>> good.  You don't talk about how many userspace applications you have
>> tested to be certain that it is actually safe to add something to this
>> file, nor do you talk about measuring performance.
>
> Makes sense. Given this and Kees comment above, it seems like status
> instead is a better place. That should deal with the compatibility
> issue given it's a key-value pair file. Do you have the same
> performance concerns for that file as well?

They are a general concern.  It is worth checking to see if the
performance of the proc file you modify changes measurably.

>> This implementation seems very fragile.  How long until you need the
>> full siginfo of the signal that caused the process to exit somewhere?
>
> For our use case probably never. I don't know if someone else will
> eventually need everything.
>
>> There are two ways to get this information with existing APIs.
>> - Catch the signal in the process and give it to someone.
>
> This would involve establishing a back-channel from the child process
> to init, which is not impossible but also not particularly
> architecturally nice.
>
>> - Debug the process and stop in PTRACE_EVENT_EXIT and read
>>   the signal with PTRACE_PEEKSIGINFO.
>
> This will not work with the SELinux rules we want to enforce on Android.
>
>> I know people have wanted the full siginfo on exit before, but we have
>> not gotten there yet.
>
> That sounds like a much bigger change. How would that look? A new
> sys-call to get the siginfo from a zombie? A new wait API?

Another proc file.  It is more that we have gotten requests for that
in the past.

I will toss out one more possibility that seems like a good solution
with existing facilities.  Have the coredump helper (aka the process
that coredumps are piped to) read the signal state from the coredump.
At which point the coredump helper can back channel to init or whatever
needs this information.

I am probably missing something obvious but the consumer of all
coredumps seems like the right place to add functionality for debugging
like this as it can tell everything about the dead userspace process.

Eric