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 <[email protected]>
Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets")
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
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
--
2.25.1
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 <[email protected]>
> Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets")
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
Tested-by: "kernelci.org bot" <[email protected]>
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-20210623.0/x86_64/x86_64_defconfig+x86-chromebook+kselftest/clang-13/lab-collabora/kselftest-lkdtm-asus-C523NA-A20057-coral.html
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: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 <[email protected]>
> > Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets")
> > Cc: [email protected]
> > Signed-off-by: Kees Cook <[email protected]>
>
>
> Tested-by: "kernelci.org bot" <[email protected]>
>
>
> 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-20210623.0/x86_64/x86_64_defconfig+x86-chromebook+kselftest/clang-13/lab-collabora/kselftest-lkdtm-asus-C523NA-A20057-coral.html
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
--
Kees Cook
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-sh-not-SHELL/20210619-105959
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 <[email protected]>
[ 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/[email protected] Intel Corporation
Thanks,
Oliver Sang
+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 <[email protected]>
>>> Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets")
>>> Cc: [email protected]
>>> Signed-off-by: Kees Cook <[email protected]>
>>
>>
>> Tested-by: "kernelci.org bot" <[email protected]>
>>
>>
>> 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-20210623.0/x86_64/x86_64_defconfig+x86-chromebook+kselftest/clang-13/lab-collabora/kselftest-lkdtm-asus-C523NA-A20057-coral.html
>
> 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 [email protected] or
myself and we can give it a spin on staging.kernelci.org as well.
Best wishes,
Guillaume
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
--
Kees Cook
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. :)
--
Kees Cook
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)