All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Peter Collingbourne <pcc@google.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	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>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Evgenii Stepanov <eugenis@google.com>
Subject: Re: [PATCH] kernel: introduce prctl(PR_LOG_UACCESS)
Date: Wed, 22 Sep 2021 19:46:47 +0200	[thread overview]
Message-ID: <29f1822d-e4cd-eedb-bea8-619db1d56335@redhat.com> (raw)
In-Reply-To: <20210922061809.736124-1-pcc@google.com>

On 22.09.21 08:18, Peter Collingbourne wrote:
> This patch introduces a kernel feature known as uaccess logging.
> With uaccess logging, the userspace program passes the address and size
> of a so-called uaccess buffer to the kernel via a prctl(). The prctl()
> is a request for the kernel to log any uaccesses made during the next
> syscall to the uaccess buffer. When the next syscall returns, the address
> one past the end of the logged uaccess buffer entries is written to the
> location specified by the third argument to the prctl(). In this way,
> the userspace program may enumerate the uaccesses logged to the access
> buffer to determine which accesses occurred.
> 
> Uaccess logging has several use cases focused around bug detection
> tools:
> 
> 1) Userspace memory safety tools such as ASan, MSan, HWASan and tools
>     making use of the ARM Memory Tagging Extension (MTE) need to monitor
>     all memory accesses in a program so that they can detect memory
>     errors. For accesses made purely in userspace, this is achieved
>     via compiler instrumentation, or for MTE, via direct hardware
>     support. However, accesses made by the kernel on behalf of the
>     user program via syscalls (i.e. uaccesses) are invisible to these
>     tools. With MTE there is some level of error detection possible in
>     the kernel (in synchronous mode, bad accesses generally result in
>     returning -EFAULT from the syscall), but by the time we get back to
>     userspace we've lost the information about the address and size of the
>     failed access, which makes it harder to produce a useful error report.
> 
>     With the current versions of the sanitizers, we address this by
>     interposing the libc syscall stubs with a wrapper that checks the
>     memory based on what we believe the uaccesses will be. However, this
>     creates a maintenance burden: each syscall must be annotated with
>     its uaccesses in order to be recognized by the sanitizer, and these
>     annotations must be continuously updated as the kernel changes. This
>     is especially burdensome for syscalls such as ioctl(2) which have a
>     large surface area of possible uaccesses.
> 
> 2) Verifying the validity of kernel accesses. This can be achieved in
>     conjunction with the userspace memory safety tools mentioned in (1).
>     Even a sanitizer whose syscall wrappers have complete knowledge of
>     the kernel's intended API may vary from the kernel's actual uaccesses
>     due to kernel bugs. A sanitizer with knowledge of the kernel's actual
>     uaccesses may produce more accurate error reports that reveal such
>     bugs.
> 
>     An example of such a bug, which was found by an earlier version of this
>     patch together with a prototype client of the API in HWASan, was fixed
>     by commit d0efb16294d1 ("net: don't unconditionally copy_from_user
>     a struct ifreq for socket ioctls"). Although this bug turned out to
>     relatively harmless, it was a bug nonetheless and it's always possible
>     that more serious bugs of this sort may be introduced in the future.
> 
> 3) Kernel fuzzing. We may use the list of reported kernel accesses to
>     guide a kernel fuzzing tool such as syzkaller (so that it knows which
>     parts of user memory to fuzz), as an alternative to providing the tool
>     with a list of syscalls and their uaccesses (which again thanks to
>     (2) may not be accurate).
> 
> 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?

Like, the application allocates a bunch of memory, issues the prctl() 
and spins in user space. What would happen if the OOM killer selects 
this task as a target and does a do_send_sig_info(SIGKILL, 
SEND_SIG_PRIV, ...) ?

-- 
Thanks,

David / dhildenb


WARNING: multiple messages have this Message-ID (diff)
From: David Hildenbrand <david@redhat.com>
To: Peter Collingbourne <pcc@google.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	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>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Evgenii Stepanov <eugenis@google.com>
Subject: Re: [PATCH] kernel: introduce prctl(PR_LOG_UACCESS)
Date: Wed, 22 Sep 2021 19:46:47 +0200	[thread overview]
Message-ID: <29f1822d-e4cd-eedb-bea8-619db1d56335@redhat.com> (raw)
In-Reply-To: <20210922061809.736124-1-pcc@google.com>

On 22.09.21 08:18, Peter Collingbourne wrote:
> This patch introduces a kernel feature known as uaccess logging.
> With uaccess logging, the userspace program passes the address and size
> of a so-called uaccess buffer to the kernel via a prctl(). The prctl()
> is a request for the kernel to log any uaccesses made during the next
> syscall to the uaccess buffer. When the next syscall returns, the address
> one past the end of the logged uaccess buffer entries is written to the
> location specified by the third argument to the prctl(). In this way,
> the userspace program may enumerate the uaccesses logged to the access
> buffer to determine which accesses occurred.
> 
> Uaccess logging has several use cases focused around bug detection
> tools:
> 
> 1) Userspace memory safety tools such as ASan, MSan, HWASan and tools
>     making use of the ARM Memory Tagging Extension (MTE) need to monitor
>     all memory accesses in a program so that they can detect memory
>     errors. For accesses made purely in userspace, this is achieved
>     via compiler instrumentation, or for MTE, via direct hardware
>     support. However, accesses made by the kernel on behalf of the
>     user program via syscalls (i.e. uaccesses) are invisible to these
>     tools. With MTE there is some level of error detection possible in
>     the kernel (in synchronous mode, bad accesses generally result in
>     returning -EFAULT from the syscall), but by the time we get back to
>     userspace we've lost the information about the address and size of the
>     failed access, which makes it harder to produce a useful error report.
> 
>     With the current versions of the sanitizers, we address this by
>     interposing the libc syscall stubs with a wrapper that checks the
>     memory based on what we believe the uaccesses will be. However, this
>     creates a maintenance burden: each syscall must be annotated with
>     its uaccesses in order to be recognized by the sanitizer, and these
>     annotations must be continuously updated as the kernel changes. This
>     is especially burdensome for syscalls such as ioctl(2) which have a
>     large surface area of possible uaccesses.
> 
> 2) Verifying the validity of kernel accesses. This can be achieved in
>     conjunction with the userspace memory safety tools mentioned in (1).
>     Even a sanitizer whose syscall wrappers have complete knowledge of
>     the kernel's intended API may vary from the kernel's actual uaccesses
>     due to kernel bugs. A sanitizer with knowledge of the kernel's actual
>     uaccesses may produce more accurate error reports that reveal such
>     bugs.
> 
>     An example of such a bug, which was found by an earlier version of this
>     patch together with a prototype client of the API in HWASan, was fixed
>     by commit d0efb16294d1 ("net: don't unconditionally copy_from_user
>     a struct ifreq for socket ioctls"). Although this bug turned out to
>     relatively harmless, it was a bug nonetheless and it's always possible
>     that more serious bugs of this sort may be introduced in the future.
> 
> 3) Kernel fuzzing. We may use the list of reported kernel accesses to
>     guide a kernel fuzzing tool such as syzkaller (so that it knows which
>     parts of user memory to fuzz), as an alternative to providing the tool
>     with a list of syscalls and their uaccesses (which again thanks to
>     (2) may not be accurate).
> 
> 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?

Like, the application allocates a bunch of memory, issues the prctl() 
and spins in user space. What would happen if the OOM killer selects 
this task as a target and does a do_send_sig_info(SIGKILL, 
SEND_SIG_PRIV, ...) ?

-- 
Thanks,

David / dhildenb


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2021-09-22 17:47 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-22  6:18 [PATCH] kernel: introduce prctl(PR_LOG_UACCESS) Peter Collingbourne
2021-09-22  6:18 ` Peter Collingbourne
2021-09-22  6:30 ` Cyrill Gorcunov
2021-09-22  6:30   ` Cyrill Gorcunov
2021-11-23  5:17   ` Peter Collingbourne
2021-11-23  5:17     ` Peter Collingbourne
2021-09-22 10:44 ` Marco Elver
2021-09-22 10:44   ` Marco Elver
2021-11-23  5:17   ` Peter Collingbourne
2021-11-23  5:17     ` Peter Collingbourne
2021-09-22 13:45 ` Dmitry Vyukov
2021-09-22 13:45   ` Dmitry Vyukov
2021-09-22 22:30   ` Peter Collingbourne
2021-09-22 22:30     ` Peter Collingbourne
2021-09-22 14:23 ` Eric W. Biederman
2021-09-22 14:23   ` Eric W. Biederman
2021-09-22 15:30   ` Kees Cook
2021-09-22 15:30     ` Kees Cook
2021-09-22 15:59     ` Jann Horn
2021-09-22 15:59       ` Jann Horn
2021-09-24 21:50       ` Peter Collingbourne
2021-09-24 21:50         ` Peter Collingbourne
2021-09-26  2:20         ` Kees Cook
2021-09-26  2:20           ` Kees Cook
2021-11-23  5:17           ` Peter Collingbourne
2021-11-23  5:17             ` Peter Collingbourne
2021-09-22 17:46 ` David Hildenbrand [this message]
2021-09-22 17:46   ` David Hildenbrand
2021-09-22 19:22   ` Steven Rostedt
2021-09-22 19:22     ` Steven Rostedt
2021-09-22 19:53     ` Peter Zijlstra
2021-09-22 19:53       ` Peter Zijlstra
2021-09-22 22:05       ` Peter Collingbourne
2021-09-22 22:05         ` Peter Collingbourne
2021-09-23  8:08     ` David Hildenbrand
2021-09-23  8:08       ` David Hildenbrand

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=29f1822d-e4cd-eedb-bea8-619db1d56335@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@gmail.com \
    --cc=arnd@arndb.de \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=caoxiaofeng@yulong.com \
    --cc=catalin.marinas@arm.com \
    --cc=chris.hyser@oracle.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=christian.brauner@ubuntu.com \
    --cc=colin.king@canonical.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dietmar.eggemann@arm.com \
    --cc=dvyukov@google.com \
    --cc=ebiederm@xmission.com \
    --cc=elver@google.com \
    --cc=eugenis@google.com \
    --cc=frederic@kernel.org \
    --cc=glider@google.com \
    --cc=gorcunov@gmail.com \
    --cc=juri.lelli@redhat.com \
    --cc=keescook@chromium.org \
    --cc=krisman@collabora.com \
    --cc=legion@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=pcc@google.com \
    --cc=peterz@infradead.org \
    --cc=ran.xiaokai@zte.com.cn \
    --cc=rostedt@goodmis.org \
    --cc=samitolvanen@google.com \
    --cc=sblbir@amazon.com \
    --cc=tglx@linutronix.de \
    --cc=thomascedeno@google.com \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    --cc=will@kernel.org \
    --cc=yifeifz2@illinois.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.