Messages in this thread | | | Date | Fri, 03 Dec 2021 08:21:50 -0800 | From | Kees Cook <> | Subject | Re: [PATCH] lib/test ubsan: Silence compile-time array bounds warnings |
| |
On December 3, 2021 2:49:53 AM PST, Marco Elver <elver@google.com> wrote: >On Fri, 3 Dec 2021 at 10:30, Kees Cook <keescook@chromium.org> wrote: >> The UBSAN tests intentionally operate beyond array bounds, so silence >> the warning visible with a -Warray-bounds build: >> >> lib/test_ubsan.c: In function 'test_ubsan_object_size_mismatch': >> lib/test_ubsan.c:109:16: error: array subscript 'long long int[0]' is partly outside array bounds of 'volatile int[1]' [-Werror=array-bounds] >> 109 | val2 = *ptr; >> | ^~~~ >> lib/test_ubsan.c:104:22: note: while referencing 'val' >> 104 | volatile int val __aligned(8) = 4; >> | ^~~ >> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> lib/Makefile | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/lib/Makefile b/lib/Makefile >> index 08959b10bac9..2742a54a4275 100644 >> --- a/lib/Makefile >> +++ b/lib/Makefile >> @@ -70,6 +70,7 @@ obj-$(CONFIG_KASAN_MODULE_TEST) += test_kasan_module.o >> CFLAGS_test_kasan_module.o += -fno-builtin >> obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o >> CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla) >> +CFLAGS_test_ubsan.o += $(call cc-disable-warning, array-bounds) >> UBSAN_SANITIZE_test_ubsan.o := y >> obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o >> obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o > >Are there other warnings or only the one for the fsanitize=object-size >test? I think this is fine if there are other warnings.
I will double check, but I think it's only the object-size test, which seems to confirm my suspicion that -Warray-bounds provides sufficient coverage and object-size can be removed.
I have another patch I intend to send today for the sk_buff/sk_buff_head issue, as -Warray-bounds warns for that as well.
>But, if it's only about the fsanitize=object-size test, I'm going to >propose something more drastic. :-)
Are there any cases where object-size does a run-time check that couldn't be done at compile time? That's the only reason I could see to keep it at this point, as -Warray-bounds can do the compile time checks.
>I had wanted to wait a bit and dig a little deeper, but I just posted >part of my analysis here: >https://bugzilla.kernel.org/show_bug.cgi?id=214861#c4
Thanks, I'll refer to that in my sk_buff patch. It seems -Warray-bounds suffers from the same conservativism about object casts, which is frustrating on the one hand since the warning can be a false positive (cast vs access), but on the other, it does call attention to fragile arrangements which maybe could do with adjustment.
>My proposal is to remove UBSAN_OBJECT_SIZE and its related tests. The >bugzilla bug goes into the details, but the TLDR is: >1. fsanitize=object-size is incomplete, >2. it should have been a compiler warning, >3. for everything else there is KASAN which detects real OOB, >4. for GCC we already disable UBSAN_OBJECT_SIZE.
And maybe: 5. -Warray-bounds provides the same coverage and is about to be enabled globally.
-- Kees Cook
| |