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
| ||
|
Date: Fri, 3 Dec 2021 17:53:15 +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 17:21, Kees Cook <keescook@...omium.org> wrote: > On December 3, 2021 2:49:53 AM PST, Marco Elver <elver@...gle.com> wrote: [...] > >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. Nice. Do you want to send the patch removing UBSAN_OBJECT_SIZE, or shall I do it? Perhaps it ties in better with the rest of your patches which I have no state of. > >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. No, I don't think so. I stared at the LLVM code several times now, because I still couldn't quite believe it myself, but I think it really doesn't do any dynamic checks. Hence, why below I say it should have been a compiler warning. As mentioned in the bugzilla bug, there's a FIXME in the LLVM code to do a dynamic check with the help of fsanitize=address, but that never happened. And that doesn't make much sense anyway if fsanitize=address (viz. KASAN for us) is already on and does checking itself. > >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. Yup, in which case the compiler warning already exists and point #2 above is moot. Thanks, -- Marco
Powered by blists - more mailing lists