linux-hardening - Re: [PATCH] lib/test_ubsan: Silence compile-time array bounds warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 3 Dec 2021 11:49:53 +0100
From:   Marco Elver <elver@...gle.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Andrey Konovalov <andreyknvl@...il.com>,
        Jiri Olsa <jolsa@...nel.org>, linux-kernel@...r.kernel.org,
        linux-hardening@...r.kernel.org
Subject: Re: [PATCH] lib/test_ubsan: Silence compile-time array bounds warnings

On Fri, 3 Dec 2021 at 10:30, Kees Cook <keescook@...omium.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@...omium.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.

But, if it's only about the fsanitize=object-size test, I'm going to
propose something more drastic. :-)

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

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.

Thanks,
-- Marco