From: Gabriel Krisman Bertazi <krisman@collabora.com>
To: Jann Horn <jannh@google.com>
Cc: Arnd Bergmann <arnd@kernel.org>,
Andy Lutomirski <luto@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
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 09:26:20 -0500 [thread overview]
Message-ID: <87h7pgqhdf.fsf@collabora.com> (raw)
In-Reply-To: <CAG48ez17CKBMO4193wxuWLRQWQ+q6EV=Qr5oTWiKivMxEi0zQw@mail.gmail.com> (Jann Horn's message of "Mon, 23 Nov 2020 15:02:10 +0100")
Jann Horn <jannh@google.com> writes:
> 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?
Hi Jan, Arnd,
That is correct. This is a copy pasta mistake. My apologies. I didn't
have a !GENERIC_ENTRY device to test, but just the ifdef would have
caught it.
--
Gabriel Krisman Bertazi
next prev parent reply other threads:[~2020-11-23 14:26 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
2020-11-23 14:26 ` Gabriel Krisman Bertazi [this message]
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=87h7pgqhdf.fsf@collabora.com \
--to=krisman@collabora.com \
--cc=andriin@fb.com \
--cc=arnd@arndb.de \
--cc=arnd@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=jannh@google.com \
--cc=keescook@chromium.org \
--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).