sub tests of lkdtm kselftest are failing about kernelci-project

[musamaanjum]

Additional information about ARRAY_BOUNDS.sh:
The tools/testing/selftests/lkdtm/config file didn't have correct configurations which were creating problems when merged with other config files. The fix was simple and has been sent under #83. Following patches were sent:
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/

There has been no acceptance or rejection of these on the mailing lists. Hopefully, they will be accepted. I've not included the stable mailing list. But now that I think about it, we need to have this patch in the stable as well. What should I do about it?

After adding these patches, ARRAY_BOUNDS.sh will pass.

Update: I've sent V2 by adding a Fixes tag. The patch should automatically be backported now.

[musamaanjum]

1/9 sub-tests have been fixed with the above patches. I'm working on the next sub-test failures.

[gctucker]

The tools/testing/selftests/lkdtm/config file didn't have correct configurations which were creating problems when merged with other config files. The fix was simple and has been sent under #83. Following patches were sent: https://lore.kernel.org/lkml/[email protected]/ https://lore.kernel.org/lkml/[email protected]/

There has been no acceptance or rejection of these on the mailing lists. Hopefully, they will be accepted. I've not included the stable mailing list. But now that I think about it, we need to have this patch in the stable as well. What should I do about it?

After adding these patches, ARRAY_BOUNDS.sh will pass.

Update: I've sent V2 by adding a Fixes tag. The patch should automatically be backported now.

FYI: The patches don't apply on top of staging-next so they're not being tested there. I guess we'll just have to wait for them to land in linux-next.

[hardboprobot]

@musamaanjum

Code seems correct. Bug isn't being triggered. On investigating __builtin_object_size further, I've found out that sometimes it doesn't give the correct size of the buffer from documentation.

According to the documentation I think it's the test that's wrong here:

type is an integer constant from 0 to 3. If the least significant bit is clear, objects are whole variables, if it is set, a closest surrounding subobject is considered the object a pointer points to. The second bit determines if maximum or minimum of remaining bytes is computed.

The definition of the memcpy inline wrapper in fortify-string.h uses type = 0, so the size considered will be from the beginning of the whole variable (target.a pointing to the beginning of the struct) until the end of the whole struct. See the example code in the docs for this particular case.

There's also a __builtin_dynamic_object_size builtin that might give a different result at runtime.

Investigating the history of that code a bit, it was originally written by Francis Laniel in drivers/misc/lkdtm/bugs.c (patch) and then moved from drivers/misc/lkdtm/bugs.c to drivers/misc/lkdtm/fortify.c (and modified) by Kees Cook (patch). But note that the original version used strcpy instead of memcpy, and the wrapper for strcpy uses __builtin_object_size with type = 1, which changes the test semantics completely.

Can you investigate this a bit further?

[musamaanjum]

A patch series has been sent: https://lore.kernel.org/linux-kselftest/[email protected]/t/#u
It'll fix problems in 4 sub-tests. I've updated the table above.

[musamaanjum]

After the previous patch, 3 new sub tests are passing: https://storage.staging.kernelci.org/kernelci/staging-next/staging-next-20220524.0/arm64/defconfig+arm64-chromebook+kselftest/gcc-10/lab-collabora/kselftest-lkdtm-mt8173-elm-hana.html

Closing this ticket.

[musamaanjum]

Additional information about FORTIFIED_SUBOBJECT.sh:
FORTIFY_SOURCE has been enabled. This sub-test has 2 sister tests (FORTIFIED_OBJECT and FORTIFIED_STRSCPY). They are passing. In this test, a crash from memcpy is expected. I'd to look about the FORTIFY_SOURCES kernel config option and then I found out that this config enables wrapper (fortify-string.h) on top of these functions. I've studied the wrapper of this function. Compiler build in function __builtin_object_size is being used to find the sizes of buffers. Code seems correct. Bug isn't being triggered. On investigating __builtin_object_size further, I've found out that sometimes it doesn't give the correct size of the buffer from documentation.

struct target {
		char a[10];
		char b[10];
	} target;

This test is expecting __builtin_object_size to calculate the size of target.a to 10. But __builtin_object_size calculates it to the size of the whole struct which is 20. Thus bug isn't triggered and the test is failing.

[musamaanjum]

@hardboprobot Thank you for looking it up. I've investigated it further.

Recently Kees has made a very detailed commit about memcpy(), ccb7ebac07f9d ("fortify: Detect struct member overflows in memcpy() at compile-time"). This commit verifies that this test case is wrong. The commit mentions that memcpy should capture this kind of overflows when type 2 is used in __builtin_object_size(). But at the time type 2 cannot be set as it'll generate a lot of false positives at this time. He'll keep on fixing it step by step.

Either the memcpy() should be replaced with strcpy() to make the test case correct, or support in memcpy() is needed which uses type 2.

[musamaanjum]

STACKLEAK_ERASING.sh:
This test needs GCC's plugins package, gcc-10-plugin-dev and GCC_PLUGIN_STACKLEAK enabled.

I'm unable to install gcc-10-plugin-dev package on my own native Bullseye Debian distro. The dependencies of this package are conflicting. I've reported it to Debian bug tracker. It is going to get noticied. There is some bigger dependencies conflict going on.

So I've tried the debian:latest docker container which is bullseye Debian. I can install gcc-10-plugin-dev package without any problem. So I've enabled the GCC_PLUGIN_STACKLEAK. But build fails again:

/usr/lib/gcc/x86_64-linux-gnu/10/plugin/include/config/i386/i386.h:2500:10: fatal error: common/config/i386/i386-cpuinfo.h: No such file or directory
 2500 | #include "common/config/i386/i386-cpuinfo.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

Looking at this error, I've found out that support for GCC Plugins is broken in Bullseye. There is a bug filed here. The workaround is possible by applying a patch. I've opened a issue for this kernelci/kernelci-core#1041. For now I'll send a kernel patch to enable GCC_PLUGIN_STACKLEAK config for LKDTM.

[kees]

For SLAB_LINEAR_OVERFLOW, if you don't want a full KASAN build, you should be able to detect this case with CONFIG_SLUB_DEBUG=y and booting with slub_debug=ZF.

[kees]

CFI_FORWARD_PROTO is going to fail unless there is an active CFI system in place of some kind. Right now this depends on arm64+Clang. In the future, this will be arch-agnostic+Clang, but for the moment, it should be safe to exclude this test.

[musamaanjum]

The previous series wasn't accepted. After looking at a months-long discussion, I'd sent one patch. It got accepted: https://lore.kernel.org/lkml/[email protected]/