From: Jann Horn <jannh@google.com>
To: Arnd Bergmann <arnd@kernel.org>,
Gabriel Krisman Bertazi <krisman@collabora.com>,
Andy Lutomirski <luto@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>
Cc: Naresh Kamboju <naresh.kamboju@linaro.org>,
open list <linux-kernel@vger.kernel.org>,
Netdev <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
lkft-triage@lists.linaro.org,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Kees Cook <keescook@chromium.org>,
Andrii Nakryiko <andriin@fb.com>,
Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
Andy Lutomirski <luto@amacapital.net>,
Sumit Semwal <sumit.semwal@linaro.org>,
Arnd Bergmann <arnd@arndb.de>, YiFei Zhu <yifeifz2@illinois.edu>
Subject: Re: [arm64] kernel BUG at kernel/seccomp.c:1309!
Date: Mon, 23 Nov 2020 15:02:10 +0100 [thread overview]
Message-ID: <CAG48ez17CKBMO4193wxuWLRQWQ+q6EV=Qr5oTWiKivMxEi0zQw@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a1Lx1MMQ3s1uWjevsi2wqFo2r=k1hhrxf1spUxEQX_Rag@mail.gmail.com>
On Mon, Nov 23, 2020 at 2:45 PM Arnd Bergmann <arnd@kernel.org> wrote:
> On Mon, Nov 23, 2020 at 12:15 PM Naresh Kamboju
> <naresh.kamboju@linaro.org> wrote:
> >
> > While booting arm64 kernel the following kernel BUG noticed on several arm64
> > devices running linux next 20201123 tag kernel.
> >
> >
> > $ git log --oneline next-20201120..next-20201123 -- kernel/seccomp.c
> > 5c5c5fa055ea Merge remote-tracking branch 'seccomp/for-next/seccomp'
> > bce6a8cba7bf Merge branch 'linus'
> > 7ef95e3dbcee Merge branch 'for-linus/seccomp' into for-next/seccomp
> > fab686eb0307 seccomp: Remove bogus __user annotations
> > 0d8315dddd28 seccomp/cache: Report cache data through /proc/pid/seccomp_cache
> > 8e01b51a31a1 seccomp/cache: Add "emulator" to check if filter is constant allow
> > f9d480b6ffbe seccomp/cache: Lookup syscall allowlist bitmap for fast path
> > 23d67a54857a seccomp: Migrate to use SYSCALL_WORK flag
> >
> >
> > Please find these easy steps to reproduce the kernel build and boot.
>
> Adding Gabriel Krisman Bertazi to Cc, as the last patch (23d67a54857a) here
> seems suspicious: it changes
>
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index 02aef2844c38..47763f3999f7 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -42,7 +42,7 @@ struct seccomp {
> extern int __secure_computing(const struct seccomp_data *sd);
> static inline int secure_computing(void)
> {
> - if (unlikely(test_thread_flag(TIF_SECCOMP)))
> + if (unlikely(test_syscall_work(SECCOMP)))
> return __secure_computing(NULL);
> return 0;
> }
>
> which is in the call chain directly before
>
> int __secure_computing(const struct seccomp_data *sd)
> {
> int mode = current->seccomp.mode;
>
> ...
> switch (mode) {
> case SECCOMP_MODE_STRICT:
> __secure_computing_strict(this_syscall); /* may call do_exit */
> return 0;
> case SECCOMP_MODE_FILTER:
> return __seccomp_filter(this_syscall, sd, false);
> default:
> BUG();
> }
> }
>
> Clearly, current->seccomp.mode is set to something other
> than SECCOMP_MODE_STRICT or SECCOMP_MODE_FILTER
> while the test_syscall_work(SECCOMP) returns true, and this
> must have not been the case earlier.
Ah, I think the problem is actually in
3136b93c3fb2b7c19e853e049203ff8f2b9dd2cd ("entry: Expose helpers to
migrate TIF to SYSCALL_WORK flag"). In the !GENERIC_ENTRY case, it
adds this code:
+#define set_syscall_work(fl) \
+ set_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+#define test_syscall_work(fl) \
+ test_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+#define clear_syscall_work(fl) \
+ clear_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+
+#define set_task_syscall_work(t, fl) \
+ set_ti_thread_flag(task_thread_info(t), TIF_##fl)
+#define test_task_syscall_work(t, fl) \
+ test_ti_thread_flag(task_thread_info(t), TIF_##fl)
+#define clear_task_syscall_work(t, fl) \
+ clear_ti_thread_flag(task_thread_info(t), TIF_##fl)
but the SYSCALL_WORK_FLAGS are not valid on !GENERIC_ENTRY, we'll mix
up (on arm64) SYSCALL_WORK_BIT_SECCOMP (==0) and TIF_SIGPENDING (==0).
As part of fixing this, it might be a good idea to put "enum
syscall_work_bit" behind a "#ifdef CONFIG_GENERIC_ENTRY" to avoid
future accidents like this?
next prev parent reply other threads:[~2020-11-23 14:02 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-23 11:15 [arm64] kernel BUG at kernel/seccomp.c:1309! Naresh Kamboju
2020-11-23 13:45 ` Arnd Bergmann
2020-11-23 14:02 ` Jann Horn [this message]
2020-11-23 14:26 ` Gabriel Krisman Bertazi
2020-11-23 15:54 ` [PATCH] entry: Fix boot for !CONFIG_GENERIC_ENTRY Gabriel Krisman Bertazi
2020-11-24 21:45 ` Kees Cook
2020-11-25 3:36 ` Naresh Kamboju
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='CAG48ez17CKBMO4193wxuWLRQWQ+q6EV=Qr5oTWiKivMxEi0zQw@mail.gmail.com' \
--to=jannh@google.com \
--cc=andriin@fb.com \
--cc=arnd@arndb.de \
--cc=arnd@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=keescook@chromium.org \
--cc=krisman@collabora.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkft-triage@lists.linaro.org \
--cc=luto@amacapital.net \
--cc=luto@kernel.org \
--cc=naresh.kamboju@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=sumit.semwal@linaro.org \
--cc=tglx@linutronix.de \
--cc=yhs@fb.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).