| Message ID | 20220217205620.2512094-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] selftests/lkdtm: add config and turn off CFI_FORWARD_PROTO | expand |
On 2/18/22 1:56 AM, Muhammad Usama Anjum wrote: > Add config options which are needed for LKDTM sub-tests. > STACKLEAK_ERASING test needs GCC_PLUGIN_STACKLEAK config. > READ_AFTER_FREE and READ_BUDDY_AFTER_FREE tests need > INIT_ON_FREE_DEFAULT_ON config. > > CFI_FORWARD_PROTO always fails as there is no active CFI system of some > kind. Turn it off for now by default until proper support. > > Cc: Kees Cook <keescook@chromium.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> Reported-by: "kernelci.org bot" <bot@kernelci.org>
Hi, Any thoughts? On 2/18/22 1:56 AM, Muhammad Usama Anjum wrote: > Add config options which are needed for LKDTM sub-tests. > STACKLEAK_ERASING test needs GCC_PLUGIN_STACKLEAK config. > READ_AFTER_FREE and READ_BUDDY_AFTER_FREE tests need > INIT_ON_FREE_DEFAULT_ON config. > > CFI_FORWARD_PROTO always fails as there is no active CFI system of some > kind. Turn it off for now by default until proper support. > > Cc: Kees Cook <keescook@chromium.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > tools/testing/selftests/lkdtm/config | 2 ++ > tools/testing/selftests/lkdtm/tests.txt | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/lkdtm/config b/tools/testing/selftests/lkdtm/config > index 46f39ee762086..adc9fa60057c5 100644 > --- a/tools/testing/selftests/lkdtm/config > +++ b/tools/testing/selftests/lkdtm/config > @@ -2,8 +2,10 @@ CONFIG_LKDTM=y > CONFIG_DEBUG_LIST=y > CONFIG_SLAB_FREELIST_HARDENED=y > CONFIG_FORTIFY_SOURCE=y > +CONFIG_GCC_PLUGIN_STACKLEAK=y > CONFIG_HARDENED_USERCOPY=y > CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y > +CONFIG_INIT_ON_FREE_DEFAULT_ON=y > CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y > CONFIG_UBSAN=y > CONFIG_UBSAN_BOUNDS=y > diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt > index 6b36b7f5dcf96..aa947b0ce1eeb 100644 > --- a/tools/testing/selftests/lkdtm/tests.txt > +++ b/tools/testing/selftests/lkdtm/tests.txt > @@ -72,7 +72,7 @@ USERCOPY_STACK_FRAME_FROM > USERCOPY_STACK_BEYOND > USERCOPY_KERNEL > STACKLEAK_ERASING OK: the rest of the thread stack is properly erased > -CFI_FORWARD_PROTO > +#CFI_FORWARD_PROTO > FORTIFIED_STRSCPY > FORTIFIED_OBJECT > FORTIFIED_SUBOBJECT
Reminder! On 2/28/22 12:06 PM, Muhammad Usama Anjum wrote: > Hi, > > Any thoughts? > > On 2/18/22 1:56 AM, Muhammad Usama Anjum wrote: >> Add config options which are needed for LKDTM sub-tests. >> STACKLEAK_ERASING test needs GCC_PLUGIN_STACKLEAK config. >> READ_AFTER_FREE and READ_BUDDY_AFTER_FREE tests need >> INIT_ON_FREE_DEFAULT_ON config. >> >> CFI_FORWARD_PROTO always fails as there is no active CFI system of some >> kind. Turn it off for now by default until proper support. >> >> Cc: Kees Cook <keescook@chromium.org> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >> --- >> tools/testing/selftests/lkdtm/config | 2 ++ >> tools/testing/selftests/lkdtm/tests.txt | 2 +- >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/lkdtm/config b/tools/testing/selftests/lkdtm/config >> index 46f39ee762086..adc9fa60057c5 100644 >> --- a/tools/testing/selftests/lkdtm/config >> +++ b/tools/testing/selftests/lkdtm/config >> @@ -2,8 +2,10 @@ CONFIG_LKDTM=y >> CONFIG_DEBUG_LIST=y >> CONFIG_SLAB_FREELIST_HARDENED=y >> CONFIG_FORTIFY_SOURCE=y >> +CONFIG_GCC_PLUGIN_STACKLEAK=y >> CONFIG_HARDENED_USERCOPY=y >> CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y >> +CONFIG_INIT_ON_FREE_DEFAULT_ON=y >> CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y >> CONFIG_UBSAN=y >> CONFIG_UBSAN_BOUNDS=y >> diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt >> index 6b36b7f5dcf96..aa947b0ce1eeb 100644 >> --- a/tools/testing/selftests/lkdtm/tests.txt >> +++ b/tools/testing/selftests/lkdtm/tests.txt >> @@ -72,7 +72,7 @@ USERCOPY_STACK_FRAME_FROM >> USERCOPY_STACK_BEYOND >> USERCOPY_KERNEL >> STACKLEAK_ERASING OK: the rest of the thread stack is properly erased >> -CFI_FORWARD_PROTO >> +#CFI_FORWARD_PROTO >> FORTIFIED_STRSCPY >> FORTIFIED_OBJECT >> FORTIFIED_SUBOBJECT
On Fri, Feb 18, 2022 at 01:56:19AM +0500, Muhammad Usama Anjum wrote: > Add config options which are needed for LKDTM sub-tests. > STACKLEAK_ERASING test needs GCC_PLUGIN_STACKLEAK config. > READ_AFTER_FREE and READ_BUDDY_AFTER_FREE tests need > INIT_ON_FREE_DEFAULT_ON config. > > CFI_FORWARD_PROTO always fails as there is no active CFI system of some > kind. Turn it off for now by default until proper support. Building under LTO Clang on arm64, this is available. What's the right way to add a CONFIG that isn't always available? -Kees > > Cc: Kees Cook <keescook@chromium.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > tools/testing/selftests/lkdtm/config | 2 ++ > tools/testing/selftests/lkdtm/tests.txt | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/lkdtm/config b/tools/testing/selftests/lkdtm/config > index 46f39ee762086..adc9fa60057c5 100644 > --- a/tools/testing/selftests/lkdtm/config > +++ b/tools/testing/selftests/lkdtm/config > @@ -2,8 +2,10 @@ CONFIG_LKDTM=y > CONFIG_DEBUG_LIST=y > CONFIG_SLAB_FREELIST_HARDENED=y > CONFIG_FORTIFY_SOURCE=y > +CONFIG_GCC_PLUGIN_STACKLEAK=y > CONFIG_HARDENED_USERCOPY=y > CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y > +CONFIG_INIT_ON_FREE_DEFAULT_ON=y > CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y > CONFIG_UBSAN=y > CONFIG_UBSAN_BOUNDS=y > diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt > index 6b36b7f5dcf96..aa947b0ce1eeb 100644 > --- a/tools/testing/selftests/lkdtm/tests.txt > +++ b/tools/testing/selftests/lkdtm/tests.txt > @@ -72,7 +72,7 @@ USERCOPY_STACK_FRAME_FROM > USERCOPY_STACK_BEYOND > USERCOPY_KERNEL > STACKLEAK_ERASING OK: the rest of the thread stack is properly erased > -CFI_FORWARD_PROTO > +#CFI_FORWARD_PROTO > FORTIFIED_STRSCPY > FORTIFIED_OBJECT > FORTIFIED_SUBOBJECT > -- > 2.30.2 >
On 3/10/22 12:22 AM, Kees Cook wrote: > On Fri, Feb 18, 2022 at 01:56:19AM +0500, Muhammad Usama Anjum wrote: >> Add config options which are needed for LKDTM sub-tests. >> STACKLEAK_ERASING test needs GCC_PLUGIN_STACKLEAK config. >> READ_AFTER_FREE and READ_BUDDY_AFTER_FREE tests need >> INIT_ON_FREE_DEFAULT_ON config. >> >> CFI_FORWARD_PROTO always fails as there is no active CFI system of some >> kind. Turn it off for now by default until proper support. > > Building under LTO Clang on arm64, this is available. What's the right > way to add a CONFIG that isn't always available? > > -Kees Yeah, as you had mentioned (https://github.com/kernelci/kernelci-project/issues/84#issuecomment-1042015431): 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. In this patch, I'm turning off CFI_FORWARD_PROTO by default here. We can re-enable it when it becomes arch agnostic. CFI_FORWARD_PROTO cannot be turned off by using a config. Please let me know your thoughts otherwise.
Any thoughts? On 3/10/22 10:21 PM, Muhammad Usama Anjum wrote: > On 3/10/22 12:22 AM, Kees Cook wrote: >> On Fri, Feb 18, 2022 at 01:56:19AM +0500, Muhammad Usama Anjum wrote: >>> Add config options which are needed for LKDTM sub-tests. >>> STACKLEAK_ERASING test needs GCC_PLUGIN_STACKLEAK config. >>> READ_AFTER_FREE and READ_BUDDY_AFTER_FREE tests need >>> INIT_ON_FREE_DEFAULT_ON config. >>> >>> CFI_FORWARD_PROTO always fails as there is no active CFI system of some >>> kind. Turn it off for now by default until proper support. >> >> Building under LTO Clang on arm64, this is available. What's the right >> way to add a CONFIG that isn't always available? >> >> -Kees > Yeah, as you had mentioned > (https://github.com/kernelci/kernelci-project/issues/84#issuecomment-1042015431): > > 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. > > In this patch, I'm turning off CFI_FORWARD_PROTO by default here. We can > re-enable it when it becomes arch agnostic. CFI_FORWARD_PROTO cannot be > turned off by using a config. Please let me know your thoughts otherwise. >
On Thu, Apr 14, 2022 at 04:31:42PM +0500, Muhammad Usama Anjum wrote: > Any thoughts? > > On 3/10/22 10:21 PM, Muhammad Usama Anjum wrote: > > On 3/10/22 12:22 AM, Kees Cook wrote: > >> On Fri, Feb 18, 2022 at 01:56:19AM +0500, Muhammad Usama Anjum wrote: > >>> Add config options which are needed for LKDTM sub-tests. > >>> STACKLEAK_ERASING test needs GCC_PLUGIN_STACKLEAK config. > >>> READ_AFTER_FREE and READ_BUDDY_AFTER_FREE tests need > >>> INIT_ON_FREE_DEFAULT_ON config. > >>> > >>> CFI_FORWARD_PROTO always fails as there is no active CFI system of some > >>> kind. Turn it off for now by default until proper support. > >> > >> Building under LTO Clang on arm64, this is available. What's the right > >> way to add a CONFIG that isn't always available? > >> > >> -Kees > > Yeah, as you had mentioned > > (https://github.com/kernelci/kernelci-project/issues/84#issuecomment-1042015431): > > > > 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. > > > > In this patch, I'm turning off CFI_FORWARD_PROTO by default here. We can > > re-enable it when it becomes arch agnostic. CFI_FORWARD_PROTO cannot be > > turned off by using a config. Please let me know your thoughts otherwise. I don't like the idea of disabling the test because this means it won't have any output in CI systems to examine at all. I'd much rather have tests that are expecting to fail to report XFAIL from the kernel instead. (See things like lkdtm_UNSET_SMEP.)
diff --git a/tools/testing/selftests/lkdtm/config b/tools/testing/selftests/lkdtm/config index 46f39ee762086..adc9fa60057c5 100644 --- a/tools/testing/selftests/lkdtm/config +++ b/tools/testing/selftests/lkdtm/config @@ -2,8 +2,10 @@ CONFIG_LKDTM=y CONFIG_DEBUG_LIST=y CONFIG_SLAB_FREELIST_HARDENED=y CONFIG_FORTIFY_SOURCE=y +CONFIG_GCC_PLUGIN_STACKLEAK=y CONFIG_HARDENED_USERCOPY=y CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y +CONFIG_INIT_ON_FREE_DEFAULT_ON=y CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y CONFIG_UBSAN=y CONFIG_UBSAN_BOUNDS=y diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt index 6b36b7f5dcf96..aa947b0ce1eeb 100644 --- a/tools/testing/selftests/lkdtm/tests.txt +++ b/tools/testing/selftests/lkdtm/tests.txt @@ -72,7 +72,7 @@ USERCOPY_STACK_FRAME_FROM USERCOPY_STACK_BEYOND USERCOPY_KERNEL STACKLEAK_ERASING OK: the rest of the thread stack is properly erased -CFI_FORWARD_PROTO +#CFI_FORWARD_PROTO FORTIFIED_STRSCPY FORTIFIED_OBJECT FORTIFIED_SUBOBJECT
Add config options which are needed for LKDTM sub-tests. STACKLEAK_ERASING test needs GCC_PLUGIN_STACKLEAK config. READ_AFTER_FREE and READ_BUDDY_AFTER_FREE tests need INIT_ON_FREE_DEFAULT_ON config. CFI_FORWARD_PROTO always fails as there is no active CFI system of some kind. Turn it off for now by default until proper support. Cc: Kees Cook <keescook@chromium.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets") Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- tools/testing/selftests/lkdtm/config | 2 ++ tools/testing/selftests/lkdtm/tests.txt | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)