Some environments do not set $SHELL when running tests. There's no need to use $SHELL here anyway, so just replace it with hard-coded path instead. Additionally avoid using bash-isms in the command, so that regular /bin/sh can be used.
Suggested-by: Guillaume Tucker guillaume.tucker@collabora.com Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") Cc: stable@vger.kernel.org Signed-off-by: Kees Cook keescook@chromium.org --- tools/testing/selftests/lkdtm/run.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh index bb7a1775307b..0f9f22ac004b 100755 --- a/tools/testing/selftests/lkdtm/run.sh +++ b/tools/testing/selftests/lkdtm/run.sh @@ -78,8 +78,9 @@ dmesg > "$DMESG"
# Most shells yell about signals and we're expecting the "cat" process # to usually be killed by the kernel. So we have to run it in a sub-shell -# and silence errors. -($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true +# to avoid terminating this script. Leave stderr alone, just in case +# something _else_ happens. +(/bin/sh -c '(echo '"$test"') | cat >'"$TRIGGER") || true
# Record and dump the results dmesg | comm --nocheck-order -13 "$DMESG" - > "$LOG" || true
On 19/06/2021 03:58, Kees Cook wrote:
Some environments do not set $SHELL when running tests. There's no need to use $SHELL here anyway, so just replace it with hard-coded path instead. Additionally avoid using bash-isms in the command, so that regular /bin/sh can be used.
Suggested-by: Guillaume Tucker guillaume.tucker@collabora.com Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") Cc: stable@vger.kernel.org Signed-off-by: Kees Cook keescook@chromium.org
Tested-by: "kernelci.org bot" bot@kernelci.org
Sample staging results with this patch applied on top of next-20210622:
https://staging.kernelci.org/test/plan/id/60d2dbdc3cfb88da0924bf41/
Full log:
https://storage.staging.kernelci.org/kernelci/staging-next/staging-next-2021...
This was tested using Debian Buster with the default shell being "dash", which doesn't support Bash-specific features.
Thanks, Guillaume
tools/testing/selftests/lkdtm/run.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh index bb7a1775307b..0f9f22ac004b 100755 --- a/tools/testing/selftests/lkdtm/run.sh +++ b/tools/testing/selftests/lkdtm/run.sh @@ -78,8 +78,9 @@ dmesg > "$DMESG" # Most shells yell about signals and we're expecting the "cat" process # to usually be killed by the kernel. So we have to run it in a sub-shell -# and silence errors. -($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true +# to avoid terminating this script. Leave stderr alone, just in case +# something _else_ happens. +(/bin/sh -c '(echo '"$test"') | cat >'"$TRIGGER") || true # Record and dump the results dmesg | comm --nocheck-order -13 "$DMESG" - > "$LOG" || true
From: Guillaume Tucker
Sent: 23 June 2021 13:40
...
diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh index bb7a1775307b..0f9f22ac004b 100755 --- a/tools/testing/selftests/lkdtm/run.sh +++ b/tools/testing/selftests/lkdtm/run.sh @@ -78,8 +78,9 @@ dmesg > "$DMESG"
# Most shells yell about signals and we're expecting the "cat" process # to usually be killed by the kernel. So we have to run it in a sub-shell -# and silence errors. -($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true +# to avoid terminating this script. Leave stderr alone, just in case +# something _else_ happens. +(/bin/sh -c '(echo '"$test"') | cat >'"$TRIGGER") || true
I was having trouble parsing that command - and I'm good at shell scripts. I think the extra subshell the 'echo' is in doesn't help. In fact, is either subshell needed? Surely: /bin/sh -c "echo '$test' | cat >$trigger" || true will work just as well?
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Wed, Jun 23, 2021 at 01:43:04PM +0000, David Laight wrote:
From: Guillaume Tucker
Sent: 23 June 2021 13:40
...
diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh index bb7a1775307b..0f9f22ac004b 100755 --- a/tools/testing/selftests/lkdtm/run.sh +++ b/tools/testing/selftests/lkdtm/run.sh @@ -78,8 +78,9 @@ dmesg > "$DMESG"
# Most shells yell about signals and we're expecting the "cat" process # to usually be killed by the kernel. So we have to run it in a sub-shell -# and silence errors. -($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true +# to avoid terminating this script. Leave stderr alone, just in case +# something _else_ happens. +(/bin/sh -c '(echo '"$test"') | cat >'"$TRIGGER") || true
I was having trouble parsing that command - and I'm good at shell scripts. I think the extra subshell the 'echo' is in doesn't help. In fact, is either subshell needed? Surely: /bin/sh -c "echo '$test' | cat >$trigger" || true will work just as well?
Ah yeah, and I just tested it to double check, it can be even simpler:
echo "$test" | /bin/sh -c "cat >$TRIGGER" || true
I'll adjust and resend...
-Kees
From: Kees Cook
Sent: 23 June 2021 17:19
On Wed, Jun 23, 2021 at 01:43:04PM +0000, David Laight wrote:
From: Guillaume Tucker
Sent: 23 June 2021 13:40
...
diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh index bb7a1775307b..0f9f22ac004b 100755 --- a/tools/testing/selftests/lkdtm/run.sh +++ b/tools/testing/selftests/lkdtm/run.sh @@ -78,8 +78,9 @@ dmesg > "$DMESG"
# Most shells yell about signals and we're expecting the "cat" process # to usually be killed by the kernel. So we have to run it in a sub-shell -# and silence errors. -($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true +# to avoid terminating this script. Leave stderr alone, just in case +# something _else_ happens. +(/bin/sh -c '(echo '"$test"') | cat >'"$TRIGGER") || true
I was having trouble parsing that command - and I'm good at shell scripts. I think the extra subshell the 'echo' is in doesn't help. In fact, is either subshell needed? Surely: /bin/sh -c "echo '$test' | cat >$trigger" || true will work just as well?
Ah yeah, and I just tested it to double check, it can be even simpler:
echo "$test" | /bin/sh -c "cat >$TRIGGER" || true
You can probably even do:
echo "$test" | /bin/sh -c cat >$TRIGGER || true
(moving the redirect to the outer shell).
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Wed, Jun 23, 2021 at 04:27:47PM +0000, David Laight wrote:
From: Kees Cook
Sent: 23 June 2021 17:19
On Wed, Jun 23, 2021 at 01:43:04PM +0000, David Laight wrote:
From: Guillaume Tucker
Sent: 23 June 2021 13:40
...
diff --git a/tools/testing/selftests/lkdtm/run.sh b/tools/testing/selftests/lkdtm/run.sh index bb7a1775307b..0f9f22ac004b 100755 --- a/tools/testing/selftests/lkdtm/run.sh +++ b/tools/testing/selftests/lkdtm/run.sh @@ -78,8 +78,9 @@ dmesg > "$DMESG"
# Most shells yell about signals and we're expecting the "cat" process # to usually be killed by the kernel. So we have to run it in a sub-shell -# and silence errors. -($SHELL -c 'cat <(echo '"$test"') >'"$TRIGGER" 2>/dev/null) || true +# to avoid terminating this script. Leave stderr alone, just in case +# something _else_ happens. +(/bin/sh -c '(echo '"$test"') | cat >'"$TRIGGER") || true
I was having trouble parsing that command - and I'm good at shell scripts. I think the extra subshell the 'echo' is in doesn't help. In fact, is either subshell needed? Surely: /bin/sh -c "echo '$test' | cat >$trigger" || true will work just as well?
Ah yeah, and I just tested it to double check, it can be even simpler:
echo "$test" | /bin/sh -c "cat >$TRIGGER" || true
You can probably even do:
echo "$test" | /bin/sh -c cat >$TRIGGER || true
(moving the redirect to the outer shell).
Actually, it looks like the "write" is already happening in the exec'd process, so this can just be:
echo "$test" | cat >$TRIGGER || true
But it still can't be:
echo "$test" >$TRIGGER
which is what I had over-engineered a solution to. :)
From: Kees Cook
Sent: 23 June 2021 23:47
On Wed, Jun 23, 2021 at 04:27:47PM +0000, David Laight wrote:
...
You can probably even do:
echo "$test" | /bin/sh -c cat >$TRIGGER || true
(moving the redirect to the outer shell).
Actually, it looks like the "write" is already happening in the exec'd process, so this can just be:
echo "$test" | cat >$TRIGGER || true
But it still can't be:
echo "$test" >$TRIGGER
which is what I had over-engineered a solution to. :)
That one fails because echo is the shell builtin. But: /bin/echo "$test" >$TRIGGER should be fine.
Quite where the original came from I not sure I want to find out.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Wed, Jun 23, 2021 at 01:39:57PM +0100, Guillaume Tucker wrote:
On 19/06/2021 03:58, Kees Cook wrote:
Some environments do not set $SHELL when running tests. There's no need to use $SHELL here anyway, so just replace it with hard-coded path instead. Additionally avoid using bash-isms in the command, so that regular /bin/sh can be used.
Suggested-by: Guillaume Tucker guillaume.tucker@collabora.com Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") Cc: stable@vger.kernel.org Signed-off-by: Kees Cook keescook@chromium.org
Tested-by: "kernelci.org bot" bot@kernelci.org
Sample staging results with this patch applied on top of next-20210622:
https://staging.kernelci.org/test/plan/id/60d2dbdc3cfb88da0924bf41/
Full log:
https://storage.staging.kernelci.org/kernelci/staging-next/staging-next-2021...
Awesome! This looks great. :)
What's needed to build these kernels will different CONFIGs? I see a bunch of things (commonly found in distro kernels) that are not set:
CONFIG_SLAB_FREELIST_HARDENED=y CONFIG_FORTIFY_SOURCE=y CONFIG_HARDENED_USERCOPY=y # CONFIG_HARDENED_USERCOPY_FALLBACK is not set
Should I add these to the kselftest "config" file for LKDTM?
Thanks again for the help with this!
-Kees
+kernelci +collabora
On 23/06/2021 15:38, Kees Cook wrote:
On Wed, Jun 23, 2021 at 01:39:57PM +0100, Guillaume Tucker wrote:
On 19/06/2021 03:58, Kees Cook wrote:
Some environments do not set $SHELL when running tests. There's no need to use $SHELL here anyway, so just replace it with hard-coded path instead. Additionally avoid using bash-isms in the command, so that regular /bin/sh can be used.
Suggested-by: Guillaume Tucker guillaume.tucker@collabora.com Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") Cc: stable@vger.kernel.org Signed-off-by: Kees Cook keescook@chromium.org
Tested-by: "kernelci.org bot" bot@kernelci.org
Sample staging results with this patch applied on top of next-20210622:
https://staging.kernelci.org/test/plan/id/60d2dbdc3cfb88da0924bf41/
Full log:
https://storage.staging.kernelci.org/kernelci/staging-next/staging-next-2021...
Awesome! This looks great. :)
What's needed to build these kernels will different CONFIGs? I see a bunch of things (commonly found in distro kernels) that are not set:
CONFIG_SLAB_FREELIST_HARDENED=y CONFIG_FORTIFY_SOURCE=y CONFIG_HARDENED_USERCOPY=y # CONFIG_HARDENED_USERCOPY_FALLBACK is not set
Should I add these to the kselftest "config" file for LKDTM?
Yes, that's the current way to do it.
KernelCI is simply concatenating all the config files found under tools/testing/selftests into one big kselftest fragment which is then merged with the defconfig. We could enable arbitrary things for KernelCI but of course it's much better to not do that and stick to what's in the kernel tree.
If you do send such a patch, please CC kernelci@groups.io or myself and we can give it a spin on staging.kernelci.org as well.
Best wishes, Guillaume
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 84d8cf25b0f80da0ac229214864654a7662ec7e4 ("[PATCH v2] selftests/lkdtm: Use /bin/sh not $SHELL") url: https://github.com/0day-ci/linux/commits/Kees-Cook/selftests-lkdtm-Use-bin-s... base: https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git next
in testcase: kernel-selftests version: kernel-selftests-x86_64-f8879e85-1_20210618 with following parameters:
group: lkdtm ucode: 0xe2
test-description: The kernel contains a set of "self tests" under the tools/testing/selftests/ directory. These are intended to be small unit tests to exercise individual code paths in the kernel. test-url: https://www.kernel.org/doc/Documentation/kselftest.txt
on test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz with 32G memory
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 51.725186] kernel BUG at drivers/misc/lkdtm/bugs.c:76! [ 51.730568] invalid opcode: 0000 [#1] PREEMPT SMP PTI [ 51.735672] CPU: 0 PID: 1427 Comm: cat Not tainted 5.13.0-rc2-00011-g84d8cf25b0f8 #1 [ 51.743465] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.8.1 12/05/2017 [ 51.750909] RIP: 0010:lkdtm_BUG (kbuild/src/consumer/drivers/misc/lkdtm/bugs.c:76) [ 51.754859] Code: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 0f 1f 44 00 00 <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 66 66 2e 0f 1f 84 00 00 00 All code ======== 0: cc int3 1: cc int3 2: cc int3 3: cc int3 4: cc int3 5: cc int3 6: cc int3 7: cc int3 8: cc int3 9: cc int3 a: cc int3 b: cc int3 c: cc int3 d: cc int3 e: cc int3 f: cc int3 10: cc int3 11: cc int3 12: cc int3 13: cc int3 14: cc int3 15: cc int3 16: cc int3 17: cc int3 18: cc int3 19: cc int3 1a: cc int3 1b: cc int3 1c: cc int3 1d: cc int3 1e: cc int3 1f: cc int3 20: cc int3 21: cc int3 22: cc int3 23: cc int3 24: cc int3 25: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) 2a:* 0f 0b ud2 <-- trapping instruction 2c: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1) 33: 00 00 00 00 37: 66 data16 38: 66 data16 39: 2e cs 3a: 0f .byte 0xf 3b: 1f (bad) 3c: 84 00 test %al,(%rax) ...
Code starting with the faulting instruction =========================================== 0: 0f 0b ud2 2: 66 66 2e 0f 1f 84 00 data16 nopw %cs:0x0(%rax,%rax,1) 9: 00 00 00 00 d: 66 data16 e: 66 data16 f: 2e cs 10: 0f .byte 0xf 11: 1f (bad) 12: 84 00 test %al,(%rax) ... [ 51.773751] RSP: 0018:ffffc900011c3e58 EFLAGS: 00010282 [ 51.779007] RAX: ffffffff81992280 RBX: 0000000000000001 RCX: 0000000000000000 [ 51.786188] RDX: 0000000000000000 RSI: ffffffff8122f88f RDI: ffffffff824c3f30 [ 51.793384] RBP: ffff888812085000 R08: 0000000000000001 R09: 0000000000000001 [ 51.800566] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000004 [ 51.807746] R13: ffffc900011c3f08 R14: ffffffff828b9198 R15: ffff8881026a74b8 [ 51.814924] FS: 00007f20e7a9b540(0000) GS:ffff88881dc00000(0000) knlGS:0000000000000000 [ 51.823068] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 51.828850] CR2: 00007f20e7711000 CR3: 0000000812094006 CR4: 00000000003706f0 [ 51.836030] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 51.843213] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 51.850396] Call Trace: [ 51.852857] direct_entry.cold (kbuild/src/consumer/drivers/misc/lkdtm/core.c:397) [ 51.856806] full_proxy_write (kbuild/src/consumer/fs/debugfs/file.c:234) [ 51.860668] vfs_write (kbuild/src/consumer/fs/read_write.c:603) [ 51.864004] ksys_write (kbuild/src/consumer/fs/read_write.c:658) [ 51.867435] do_syscall_64 (kbuild/src/consumer/arch/x86/entry/common.c:47) [ 51.871035] entry_SYSCALL_64_after_hwframe (kbuild/src/consumer/arch/x86/entry/entry_64.S:112) [ 51.876122] RIP: 0033:0x7f20e79c3504 [ 51.879720] Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00 48 8d 05 f9 61 0d 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 49 89 d4 55 48 89 f5 53 All code ======== 0: 00 f7 add %dh,%bh 2: d8 64 89 02 fsubs 0x2(%rcx,%rcx,4) 6: 48 c7 c0 ff ff ff ff mov $0xffffffffffffffff,%rax d: eb b3 jmp 0xffffffffffffffc2 f: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) 16: 48 8d 05 f9 61 0d 00 lea 0xd61f9(%rip),%rax # 0xd6216 1d: 8b 00 mov (%rax),%eax 1f: 85 c0 test %eax,%eax 21: 75 13 jne 0x36 23: b8 01 00 00 00 mov $0x1,%eax 28: 0f 05 syscall 2a:* 48 3d 00 f0 ff ff cmp $0xfffffffffffff000,%rax <-- trapping instruction 30: 77 54 ja 0x86 32: c3 retq 33: 0f 1f 00 nopl (%rax) 36: 41 54 push %r12 38: 49 89 d4 mov %rdx,%r12 3b: 55 push %rbp 3c: 48 89 f5 mov %rsi,%rbp 3f: 53 push %rbx
Code starting with the faulting instruction =========================================== 0: 48 3d 00 f0 ff ff cmp $0xfffffffffffff000,%rax 6: 77 54 ja 0x5c 8: c3 retq 9: 0f 1f 00 nopl (%rax) c: 41 54 push %r12 e: 49 89 d4 mov %rdx,%r12 11: 55 push %rbp 12: 48 89 f5 mov %rsi,%rbp 15: 53 push %rbx
To reproduce:
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp install job.yaml # job file is attached in this email bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run bin/lkp run generated-yaml-file
--- 0DAY/LKP+ Test Infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/lkp@lists.01.org Intel Corporation
Thanks, Oliver Sang
On Wed, Jun 23, 2021 at 10:35:50PM +0800, kernel test robot wrote:
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 84d8cf25b0f80da0ac229214864654a7662ec7e4 ("[PATCH v2] selftests/lkdtm: Use /bin/sh not $SHELL") url: https://github.com/0day-ci/linux/commits/Kees-Cook/selftests-lkdtm-Use-bin-s... base: https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git next
in testcase: kernel-selftests version: kernel-selftests-x86_64-f8879e85-1_20210618 with following parameters:
group: lkdtm ucode: 0xe2
Heh. Yes, this is working as intended. :) Most of the lkdtm tests will trigger Oopses, and this is by design: it is checking that the kernel catches bad conditions and freaks out appropriately.
-Kees
linux-stable-mirror@lists.linaro.org
-
David Laight -
Guillaume Tucker -
Kees Cook -
kernel test robot