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.
step to reproduce:
# please install tuxmake
# sudo pip3 install -U tuxmake
# cd linux-next
# tuxmake --runtime docker --target-arch arm --toolchain gcc-9
--kconfig defconfig --kconfig-add
https://builds.tuxbuild.com/1kgWN61pS5M35vjnVfDSvOOPd38/config
# Boot the arm64 on any arm64 devices.
# you will notice the below BUG
crash log details:
-----------------------
[ 6.941012] ------------[ cut here ]------------
Found device /dev/ttyAMA3.
[ 6.947587] lima f4080000.gpu: mod rate = 500000000
[ 6.955422] kernel BUG at kernel/seccomp.c:1309!
[ 6.955430] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[ 6.955437] Modules linked in: cec rfkill wlcore_sdio(+) kirin_drm
dw_drm_dsi lima(+) drm_kms_helper gpu_sched drm fuse
[ 6.955481] CPU: 2 PID: 291 Comm: systemd-udevd Not tainted
5.10.0-rc4-next-20201123 #2
[ 6.955485] Hardware name: HiKey Development Board (DT)
[ 6.955493] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
[ 6.955510] pc : __secure_computing+0xe0/0xe8
[ 6.958171] mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot
req 400000Hz, actual 400000HZ div = 31)
[ 6.965975] [drm] Initialized lima 1.1.0 20191231 for f4080000.gpu on minor 0
[ 6.970176] lr : syscall_trace_enter+0x1cc/0x218
[ 6.970181] sp : ffff800012d8be10
[ 6.970185] x29: ffff800012d8be10 x28: ffff00000092cb00
[ 6.970195] x27: 0000000000000000 x26: 0000000000000000
[ 6.970203] x25: 0000000000000000 x24: 0000000000000000
[ 6.970210] x23: 0000000060000000 x22: 0000000000000202
[ 7.011614] mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot
req 25000000Hz, actual 24800000HZ div = 0)
[ 7.016457]
[ 7.016461] x21: 0000000000000200 x20: ffff00000092cb00
[ 7.016470] x19: ffff800012d8bec0 x18: 0000000000000000
[ 7.016478] x17: 0000000000000000 x16: 0000000000000000
[ 7.016485] x15: 0000000000000000 x14: 0000000000000000
[ 7.054116] mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot
req 400000Hz, actual 400000HZ div = 31)
[ 7.056715]
[ 7.103444] mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot
req 25000000Hz, actual 24800000HZ div = 0)
[ 7.105105] x13: 0000000000000000 x12: 0000000000000000
[ 7.125849] x11: 0000000000000000 x10: 0000000000000000
[ 7.125858] x9 : ffff80001001bcbc x8 : 0000000000000000
[ 7.125865] x7 : 0000000000000000 x6 : 0000000000000000
[ 7.125871] x5 : 0000000000000000 x4 : 0000000000000000
[ 7.125879] x3 : 0000000000000000 x2 : ffff00000092cb00
[ 7.125886] x1 : 0000000000000000 x0 : 0000000000000116
[ 7.125896] Call trace:
] Found device /dev/ttyAMA2.
[ 7.125908] __secure_computing+0xe0/0xe8
[ 7.125918] syscall_trace_enter+0x1cc/0x218
[ 7.125927] el0_svc_common.constprop.0+0x19c/0x1b8
[ 7.125933] do_el0_svc+0x2c/0x98
[ 7.125940] el0_sync_handler+0x180/0x188
[ 7.125946] el0_sync+0x174/0x180
[ 7.125958] Code: d2800121 97ffd9a9 d2800120 97fbf1a9 (d4210000)
[ 7.199584] ---[ end trace 463debbc21f0c7b5 ]---
[ 7.204205] note: systemd-udevd[291] exited with preempt_count 1
[ 7.210733] ------------[ cut here ]------------
[ 7.215451] WARNING: CPU: 2 PID: #
0 at kernel/rcu/tree.c:632 rcu_eqs_enter.isra.0+0x134/0x140
[ 7.223927] Modules linked in: cec rfkill wlcore_sdio kirin_drm
dw_drm_dsi lima drm_kms_helper gpu_sched drm fuse
[ 7.234295] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G D
5.10.0-rc4-next-20201123 #2
[ 7.243252] Hardware name: HiKey Development Board (DT)
[ 7.248561] pstate: 200003c5 (nzCv DAIF -PAN -UAO -TCO BTYPE=--)
[ 7.254638] pc : rcu_eqs_enter.isra.0+0x134/0x140
[ 7.259350] lr : rcu_idle_enter+0x18/0x28
[ 7.263362] sp : ffff8000128e3e80
[ 7.266678] x29: ffff8000128e3e80 x28: 0000000000000000
[ 7.272001] x27: 0000000000000000 x26: ffff000001b79080
[ 7.277321] x25: 0000000000000000 x24: 00000001adc9b310
[ 7.282641] x23: 0000000000000000 x22: ffff000001b79080
[ 7.287970] x21: ffff000077b24b00 x20: ffff000001b79098
[ 7.287979] x19: ffff800011c7ab40 x18: 0000000000000010
[ 7.287986] x17: 0000000000000000 x16: 0000000000000000
[ 7.287993] x15: ffff00000092cf98 x14: 0720072007200720
[ 7.288001] x13: 0720072007200720 x12: 00000000000003c6
[ 7.288008] x11: 071c71c71c71c71c x10: 00000000000003c6
[ 7.288016] x9 : ffff800010df267c x8 : 000000000000048c
[ 7.288023] x7 : 0000000000000c6f x6 : 0000000000009c3f
[ 7.288030] x5 : 00000000ffffffff x4 : 0000000000000015
[ 7.288038] x3 : 000000000022b7f0 x2 : 4000000000000002
[ 7.288046] x1 : 4000000000000000 x0 : ffff000077b26b40
[ 7.288054] Call trace:
[ 7.288064] rcu_eqs_enter.isra.0+0x134/0x140
#
[ 7.288069] rcu_idle_enter+0x18/0x28
[ 7.288078] cpuidle_enter_state+0x34c/0x438
[ 7.288084] cpuidle_enter+0x40/0x58
[ 7.288092] call_cpuidle+0x24/0x50
Reached target Sockets.
[ 7.288108] do_idle+0x228/0x290
[ 7.375468] cpu_startup_entry+0x30/0x78
[ 7.379397] secondary_start_kernel+0x158/0x190
[ 7.383930] ---[ end trace 463debbc21f0c7b6 ]---
[ OK ] Reached target B#
Reported-by: Naresh Kamboju <[email protected]>
full test log,
https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20201123/testrun/3478150/suite/linux-log-parser/test/check-kernel-bug-1968549/log
https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20201123/testrun/3478177/suite/linux-log-parser/test/check-kernel-bug-1968583/log
https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20201123/testrun/3478197/suite/linux-log-parser/test/check-kernel-bug-147858/log
metadata:
git branch: master
git repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
git commit: 62918e6fd7b5751c1285c7f8c6cbd27eb6600c02
git describe: next-20201123
make_kernelversion: 5.10.0-rc4
kernel-config: https://builds.tuxbuild.com/1kgWN61pS5M35vjnVfDSvOOPd38/config
--
Linaro LKFT
https://lkft.linaro.org
On Mon, Nov 23, 2020 at 12:15 PM Naresh Kamboju
<[email protected]> 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.
Arnd
>
> step to reproduce:
> # please install tuxmake
> # sudo pip3 install -U tuxmake
> # cd linux-next
> # tuxmake --runtime docker --target-arch arm --toolchain gcc-9
> --kconfig defconfig --kconfig-add
> https://builds.tuxbuild.com/1kgWN61pS5M35vjnVfDSvOOPd38/config
>
> # Boot the arm64 on any arm64 devices.
> # you will notice the below BUG
>
> crash log details:
> -----------------------
> [ 6.941012] ------------[ cut here ]------------
> Found device /dev/ttyAMA3.
> [ 6.947587] lima f4080000.gpu: mod rate = 500000000
> [ 6.955422] kernel BUG at kernel/seccomp.c:1309!
> [ 6.955430] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [ 6.955437] Modules linked in: cec rfkill wlcore_sdio(+) kirin_drm
> dw_drm_dsi lima(+) drm_kms_helper gpu_sched drm fuse
> [ 6.955481] CPU: 2 PID: 291 Comm: systemd-udevd Not tainted
> 5.10.0-rc4-next-20201123 #2
> [ 6.955485] Hardware name: HiKey Development Board (DT)
> [ 6.955493] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> [ 6.955510] pc : __secure_computing+0xe0/0xe8
> [ 6.958171] mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot
> req 400000Hz, actual 400000HZ div = 31)
> [ 6.965975] [drm] Initialized lima 1.1.0 20191231 for f4080000.gpu on minor 0
> [ 6.970176] lr : syscall_trace_enter+0x1cc/0x218
> [ 6.970181] sp : ffff800012d8be10
> [ 6.970185] x29: ffff800012d8be10 x28: ffff00000092cb00
> [ 6.970195] x27: 0000000000000000 x26: 0000000000000000
> [ 6.970203] x25: 0000000000000000 x24: 0000000000000000
> [ 6.970210] x23: 0000000060000000 x22: 0000000000000202
> [ 7.011614] mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot
> req 25000000Hz, actual 24800000HZ div = 0)
> [ 7.016457]
> [ 7.016461] x21: 0000000000000200 x20: ffff00000092cb00
> [ 7.016470] x19: ffff800012d8bec0 x18: 0000000000000000
> [ 7.016478] x17: 0000000000000000 x16: 0000000000000000
> [ 7.016485] x15: 0000000000000000 x14: 0000000000000000
> [ 7.054116] mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot
> req 400000Hz, actual 400000HZ div = 31)
> [ 7.056715]
> [ 7.103444] mmc_host mmc2: Bus speed (slot 0) = 24800000Hz (slot
> req 25000000Hz, actual 24800000HZ div = 0)
> [ 7.105105] x13: 0000000000000000 x12: 0000000000000000
> [ 7.125849] x11: 0000000000000000 x10: 0000000000000000
> [ 7.125858] x9 : ffff80001001bcbc x8 : 0000000000000000
> [ 7.125865] x7 : 0000000000000000 x6 : 0000000000000000
> [ 7.125871] x5 : 0000000000000000 x4 : 0000000000000000
> [ 7.125879] x3 : 0000000000000000 x2 : ffff00000092cb00
> [ 7.125886] x1 : 0000000000000000 x0 : 0000000000000116
> [ 7.125896] Call trace:
> ] Found device /dev/ttyAMA2.
> [ 7.125908] __secure_computing+0xe0/0xe8
> [ 7.125918] syscall_trace_enter+0x1cc/0x218
> [ 7.125927] el0_svc_common.constprop.0+0x19c/0x1b8
> [ 7.125933] do_el0_svc+0x2c/0x98
> [ 7.125940] el0_sync_handler+0x180/0x188
> [ 7.125946] el0_sync+0x174/0x180
> [ 7.125958] Code: d2800121 97ffd9a9 d2800120 97fbf1a9 (d4210000)
> [ 7.199584] ---[ end trace 463debbc21f0c7b5 ]---
> [ 7.204205] note: systemd-udevd[291] exited with preempt_count 1
> [ 7.210733] ------------[ cut here ]------------
> [ 7.215451] WARNING: CPU: 2 PID: #
> 0 at kernel/rcu/tree.c:632 rcu_eqs_enter.isra.0+0x134/0x140
> [ 7.223927] Modules linked in: cec rfkill wlcore_sdio kirin_drm
> dw_drm_dsi lima drm_kms_helper gpu_sched drm fuse
> [ 7.234295] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G D
> 5.10.0-rc4-next-20201123 #2
> [ 7.243252] Hardware name: HiKey Development Board (DT)
> [ 7.248561] pstate: 200003c5 (nzCv DAIF -PAN -UAO -TCO BTYPE=--)
> [ 7.254638] pc : rcu_eqs_enter.isra.0+0x134/0x140
> [ 7.259350] lr : rcu_idle_enter+0x18/0x28
> [ 7.263362] sp : ffff8000128e3e80
> [ 7.266678] x29: ffff8000128e3e80 x28: 0000000000000000
> [ 7.272001] x27: 0000000000000000 x26: ffff000001b79080
> [ 7.277321] x25: 0000000000000000 x24: 00000001adc9b310
> [ 7.282641] x23: 0000000000000000 x22: ffff000001b79080
> [ 7.287970] x21: ffff000077b24b00 x20: ffff000001b79098
> [ 7.287979] x19: ffff800011c7ab40 x18: 0000000000000010
> [ 7.287986] x17: 0000000000000000 x16: 0000000000000000
> [ 7.287993] x15: ffff00000092cf98 x14: 0720072007200720
> [ 7.288001] x13: 0720072007200720 x12: 00000000000003c6
> [ 7.288008] x11: 071c71c71c71c71c x10: 00000000000003c6
> [ 7.288016] x9 : ffff800010df267c x8 : 000000000000048c
> [ 7.288023] x7 : 0000000000000c6f x6 : 0000000000009c3f
> [ 7.288030] x5 : 00000000ffffffff x4 : 0000000000000015
> [ 7.288038] x3 : 000000000022b7f0 x2 : 4000000000000002
> [ 7.288046] x1 : 4000000000000000 x0 : ffff000077b26b40
> [ 7.288054] Call trace:
> [ 7.288064] rcu_eqs_enter.isra.0+0x134/0x140
> #
> [ 7.288069] rcu_idle_enter+0x18/0x28
> [ 7.288078] cpuidle_enter_state+0x34c/0x438
> [ 7.288084] cpuidle_enter+0x40/0x58
> [ 7.288092] call_cpuidle+0x24/0x50
> Reached target Sockets.
> [ 7.288108] do_idle+0x228/0x290
> [ 7.375468] cpu_startup_entry+0x30/0x78
> [ 7.379397] secondary_start_kernel+0x158/0x190
> [ 7.383930] ---[ end trace 463debbc21f0c7b6 ]---
> [ OK ] Reached target B#
>
> Reported-by: Naresh Kamboju <[email protected]>
>
> full test log,
> https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20201123/testrun/3478150/suite/linux-log-parser/test/check-kernel-bug-1968549/log
> https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20201123/testrun/3478177/suite/linux-log-parser/test/check-kernel-bug-1968583/log
> https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20201123/testrun/3478197/suite/linux-log-parser/test/check-kernel-bug-147858/log
>
> metadata:
> git branch: master
> git repo: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next
> git commit: 62918e6fd7b5751c1285c7f8c6cbd27eb6600c02
> git describe: next-20201123
> make_kernelversion: 5.10.0-rc4
> kernel-config: https://builds.tuxbuild.com/1kgWN61pS5M35vjnVfDSvOOPd38/config
>
>
> --
> Linaro LKFT
> https://lkft.linaro.org
On Mon, Nov 23, 2020 at 2:45 PM Arnd Bergmann <[email protected]> wrote:
> On Mon, Nov 23, 2020 at 12:15 PM Naresh Kamboju
> <[email protected]> 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?
Jann Horn <[email protected]> writes:
> On Mon, Nov 23, 2020 at 2:45 PM Arnd Bergmann <[email protected]> wrote:
>> On Mon, Nov 23, 2020 at 12:15 PM Naresh Kamboju
>> <[email protected]> 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
Gabriel Krisman Bertazi <[email protected]> writes:
> Jann Horn <[email protected]> writes:
>> 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.
I have patched it as suggested. Tested on qemu for arm32 and on bare
metal for x86-64.
Once again, my apologies for the mistake.
-- >8 --
Subject: [PATCH] entry: Fix boot for !CONFIG_GENERIC_ENTRY
A copy-pasta mistake tries to set SYSCALL_WORK flags instead of TIF
flags for !CONFIG_GENERIC_ENTRY. Also, add safeguards to catch this at
compilation time.
Reported-by: Naresh Kamboju <[email protected]>
Suggested-by: Jann Horn <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
include/linux/thread_info.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 6a597fd5d351..45ad3176e2fa 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -35,6 +35,7 @@ enum {
GOOD_STACK,
};
+#ifdef CONFIG_GENERIC_ENTRY
enum syscall_work_bit {
SYSCALL_WORK_BIT_SECCOMP,
SYSCALL_WORK_BIT_SYSCALL_TRACEPOINT,
@@ -48,6 +49,7 @@ enum syscall_work_bit {
#define SYSCALL_WORK_SYSCALL_TRACE BIT(SYSCALL_WORK_BIT_SYSCALL_TRACE)
#define SYSCALL_WORK_SYSCALL_EMU BIT(SYSCALL_WORK_BIT_SYSCALL_EMU)
#define SYSCALL_WORK_SYSCALL_AUDIT BIT(SYSCALL_WORK_BIT_SYSCALL_AUDIT)
+#endif
#include <asm/thread_info.h>
@@ -127,11 +129,11 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
clear_bit(SYSCALL_WORK_BIT_##fl, &task_thread_info(t)->syscall_work)
#else
#define set_syscall_work(fl) \
- set_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ set_ti_thread_flag(current_thread_info(), TIF_##fl)
#define test_syscall_work(fl) \
- test_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ test_ti_thread_flag(current_thread_info(), TIF_##fl)
#define clear_syscall_work(fl) \
- clear_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ clear_ti_thread_flag(current_thread_info(), TIF_##fl)
#define set_task_syscall_work(t, fl) \
set_ti_thread_flag(task_thread_info(t), TIF_##fl)
--
2.29.2
On Mon, Nov 23, 2020 at 10:54:58AM -0500, Gabriel Krisman Bertazi wrote:
> Gabriel Krisman Bertazi <[email protected]> writes:
>
> > Jann Horn <[email protected]> writes:
> >> 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.
>
> I have patched it as suggested. Tested on qemu for arm32 and on bare
> metal for x86-64.
>
> Once again, my apologies for the mistake.
>
> -- >8 --
> Subject: [PATCH] entry: Fix boot for !CONFIG_GENERIC_ENTRY
>
> A copy-pasta mistake tries to set SYSCALL_WORK flags instead of TIF
> flags for !CONFIG_GENERIC_ENTRY. Also, add safeguards to catch this at
> compilation time.
>
> Reported-by: Naresh Kamboju <[email protected]>
> Suggested-by: Jann Horn <[email protected]>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
Thanks for getting this fixed!
3136b93c3fb2 ("entry: Expose helpers to migrate TIF to SYSCALL_WORK flags")
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
The following commit has been merged into the core/entry branch of tip:
Commit-ID: 0395124a2fbff5132afee5767071ebe7e05885ac
Gitweb: https://git.kernel.org/tip/0395124a2fbff5132afee5767071ebe7e05885ac
Author: Gabriel Krisman Bertazi <[email protected]>
AuthorDate: Mon, 23 Nov 2020 10:54:58 -05:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 24 Nov 2020 23:44:28 +01:00
entry: Fix boot for !CONFIG_GENERIC_ENTRY
A copy-pasta mistake tries to set SYSCALL_WORK flags instead of TIF
flags for !CONFIG_GENERIC_ENTRY. Also, add safeguards to catch this at
compilation time.
Fixes: 3136b93c3fb2 ("entry: Expose helpers to migrate TIF to SYSCALL_WORK flags")
Reported-by: Naresh Kamboju <[email protected]>
Suggested-by: Jann Horn <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
include/linux/thread_info.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 3173632..5515ab3 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -35,6 +35,7 @@ enum {
GOOD_STACK,
};
+#ifdef CONFIG_GENERIC_ENTRY
enum syscall_work_bit {
SYSCALL_WORK_BIT_SECCOMP,
SYSCALL_WORK_BIT_SYSCALL_TRACEPOINT,
@@ -48,6 +49,7 @@ enum syscall_work_bit {
#define SYSCALL_WORK_SYSCALL_TRACE BIT(SYSCALL_WORK_BIT_SYSCALL_TRACE)
#define SYSCALL_WORK_SYSCALL_EMU BIT(SYSCALL_WORK_BIT_SYSCALL_EMU)
#define SYSCALL_WORK_SYSCALL_AUDIT BIT(SYSCALL_WORK_BIT_SYSCALL_AUDIT)
+#endif
#include <asm/thread_info.h>
@@ -129,11 +131,11 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
#else /* CONFIG_GENERIC_ENTRY */
#define set_syscall_work(fl) \
- set_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ set_ti_thread_flag(current_thread_info(), TIF_WORK_##fl)
#define test_syscall_work(fl) \
- test_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ test_ti_thread_flag(current_thread_info(), TIF_WORK_##fl)
#define clear_syscall_work(fl) \
- clear_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ clear_ti_thread_flag(current_thread_info(), TIF_WORK_##fl)
#define set_task_syscall_work(t, fl) \
set_ti_thread_flag(task_thread_info(t), TIF_##fl)
The following commit has been merged into the core/entry branch of tip:
Commit-ID: 5903f61e035320104394f721f74cd22171636f63
Gitweb: https://git.kernel.org/tip/5903f61e035320104394f721f74cd22171636f63
Author: Gabriel Krisman Bertazi <[email protected]>
AuthorDate: Mon, 23 Nov 2020 10:54:58 -05:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 25 Nov 2020 02:20:09 +01:00
entry: Fix boot for !CONFIG_GENERIC_ENTRY
A copy-pasta mistake tries to set SYSCALL_WORK flags instead of TIF
flags for !CONFIG_GENERIC_ENTRY. Also, add safeguards to catch this at
compilation time.
Fixes: 3136b93c3fb2 ("entry: Expose helpers to migrate TIF to SYSCALL_WORK flags")
Reported-by: Naresh Kamboju <[email protected]>
Suggested-by: Jann Horn <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
include/linux/thread_info.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 3173632..ca80a21 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -35,6 +35,7 @@ enum {
GOOD_STACK,
};
+#ifdef CONFIG_GENERIC_ENTRY
enum syscall_work_bit {
SYSCALL_WORK_BIT_SECCOMP,
SYSCALL_WORK_BIT_SYSCALL_TRACEPOINT,
@@ -48,6 +49,7 @@ enum syscall_work_bit {
#define SYSCALL_WORK_SYSCALL_TRACE BIT(SYSCALL_WORK_BIT_SYSCALL_TRACE)
#define SYSCALL_WORK_SYSCALL_EMU BIT(SYSCALL_WORK_BIT_SYSCALL_EMU)
#define SYSCALL_WORK_SYSCALL_AUDIT BIT(SYSCALL_WORK_BIT_SYSCALL_AUDIT)
+#endif
#include <asm/thread_info.h>
@@ -129,11 +131,11 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
#else /* CONFIG_GENERIC_ENTRY */
#define set_syscall_work(fl) \
- set_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ set_ti_thread_flag(current_thread_info(), TIF_##fl)
#define test_syscall_work(fl) \
- test_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ test_ti_thread_flag(current_thread_info(), TIF_##fl)
#define clear_syscall_work(fl) \
- clear_ti_thread_flag(current_thread_info(), SYSCALL_WORK_##fl)
+ clear_ti_thread_flag(current_thread_info(), TIF_##fl)
#define set_task_syscall_work(t, fl) \
set_ti_thread_flag(task_thread_info(t), TIF_##fl)
On Wed, 25 Nov 2020 at 03:15, Kees Cook <[email protected]> wrote:
>
> On Mon, Nov 23, 2020 at 10:54:58AM -0500, Gabriel Krisman Bertazi wrote:
> > Gabriel Krisman Bertazi <[email protected]> writes:
> >
> > > Jann Horn <[email protected]> writes:
> > >> 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.
> >
> > I have patched it as suggested. Tested on qemu for arm32 and on bare
> > metal for x86-64.
> >
> > Once again, my apologies for the mistake.
> >
> > -- >8 --
> > Subject: [PATCH] entry: Fix boot for !CONFIG_GENERIC_ENTRY
> >
> > A copy-pasta mistake tries to set SYSCALL_WORK flags instead of TIF
> > flags for !CONFIG_GENERIC_ENTRY. Also, add safeguards to catch this at
> > compilation time.
This patch tested on arm64, arm, x86_64 and i386
and the reported issue got fixed.
> >
> > Reported-by: Naresh Kamboju <[email protected]>
> > Suggested-by: Jann Horn <[email protected]>
> > Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
>
> Thanks for getting this fixed!
>
> 3136b93c3fb2 ("entry: Expose helpers to migrate TIF to SYSCALL_WORK flags")
> Reviewed-by: Kees Cook <[email protected]>
Tested-by: Naresh Kamboju <[email protected]>
- Naresh