Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210922061809.736124-1-pcc@google.com> <29f1822d-e4cd-eedb-bea8-619db1d56335@redhat.com>
 <20210922152250.4e7c869a@gandalf.local.home> <YUuJo7IfS4x0iLfX@hirez.programming.kicks-ass.net>
In-Reply-To: <YUuJo7IfS4x0iLfX@hirez.programming.kicks-ass.net>
From:   Peter Collingbourne <pcc@google.com>
Date:   Wed, 22 Sep 2021 15:05:46 -0700
Message-ID: <CAMn1gO7L=K7xRxi6yrkr+7BWZN_9UEprt=ywbvQ7M-0HVgf2ow@mail.gmail.com>
Subject: Re: [PATCH] kernel: introduce prctl(PR_LOG_UACCESS)
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Steven Rostedt <rostedt@goodmis.org>,
        David Hildenbrand <david@redhat.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>, Ingo Molnar <mingo@redhat.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Andy Lutomirski <luto@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Sami Tolvanen <samitolvanen@google.com>,
        YiFei Zhu <yifeifz2@illinois.edu>,
        Colin Ian King <colin.king@canonical.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Frederic Weisbecker <frederic@kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Andrey Konovalov <andreyknvl@gmail.com>,
        Gabriel Krisman Bertazi <krisman@collabora.com>,
        Balbir Singh <sblbir@amazon.com>,
        Chris Hyser <chris.hyser@oracle.com>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        Chris Wilson <chris@chris-wilson.co.uk>,
        Arnd Bergmann <arnd@arndb.de>,
        Dmitry Vyukov <dvyukov@google.com>,
        Christian Brauner <christian.brauner@ubuntu.com>,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        Alexey Gladkov <legion@kernel.org>,
        Ran Xiaokai <ran.xiaokai@zte.com.cn>,
        Xiaofeng Cao <caoxiaofeng@yulong.com>,
        Cyrill Gorcunov <gorcunov@gmail.com>,
        Thomas Cedeno <thomascedeno@google.com>,
        Marco Elver <elver@google.com>,
        Alexander Potapenko <glider@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Evgenii Stepanov <eugenis@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Sep 22, 2021 at 12:56 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Sep 22, 2021 at 03:22:50PM -0400, Steven Rostedt wrote:
> > On Wed, 22 Sep 2021 19:46:47 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >
> > > > All signals except SIGKILL and SIGSTOP are masked for the interval
> > > > between the prctl() and the next syscall in order to prevent handlers
> > > > for intervening asynchronous signals from issuing syscalls that may
> > > > cause uaccesses from the wrong syscall to be logged.
> > >
> > > Stupid question: can this be exploited from user space to effectively
> > > disable SIGKILL for a long time ... and do we care?
> >
> > I first misread it too, but then caught my mistake reading it a second
> > time. It says "except SIGKILL". So no, it does not disable SIGKILL.
>
> Disabling SIGINT might already be a giant nuisance. Letting through
> SIGSTOP but not SIGCONT seems awkward. Blocking SIGTRAP seems like a bad
> idea too. Blocking SIGBUS as delivered by #MC will be hillarious.

I'm only blocking the signals that are already blockable from
userspace via rt_sigprocmask (which prevents blocking SIGKILL and
SIGSTOP, but allows blocking the others including SIGBUS, for which
the man page states that the result is undefined if synchronously
generated while blocked). So in terms of blocking signals I don't
think this is exposing any new capabilities.

Per the sigaction man page, SIGKILL and SIGSTOP can't have userspace
signal handlers, so we don't need to block them in order to prevent
intervening asynchronous signal handlers (nor do we want to due to the
DoS potential). I would need to double check the behavior but I
believe that for SIGCONT continuing the process is separate from
signal delivery and unaffected by blocking (see prepare_signal in
kernel/signal.c) -- so the SIGCONT will make it continue and the
handler if any would be called once the syscall returns after the
automatic restoration of the signal mask.

Peter