linux-kernel - Re: [PATCH v2] x86/traps: Enable UBSAN traps on x86

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] [day] [month] [year] [list]

Date: Tue, 18 Jun 2024 01:57:44 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Kees Cook <kees@...nel.org>
Cc: Gatlin Newhouse <gatlin.newhouse@...il.com>, Ingo Molnar
 <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, Dave Hansen
 <dave.hansen@...ux.intel.com>, x86@...nel.org, "H. Peter Anvin"
 <hpa@...or.com>, Marco Elver <elver@...gle.com>, Andrey Konovalov
 <andreyknvl@...il.com>, Andrey Ryabinin <ryabinin.a.a@...il.com>, Nathan
 Chancellor <nathan@...nel.org>, Nick Desaulniers
 <ndesaulniers@...gle.com>, Bill Wendling <morbo@...gle.com>, Justin Stitt
 <justinstitt@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>, Rick
 Edgecombe <rick.p.edgecombe@...el.com>, Baoquan He <bhe@...hat.com>,
 Changbin Du <changbin.du@...wei.com>, Pengfei Xu <pengfei.xu@...el.com>,
 Josh Poimboeuf <jpoimboe@...nel.org>, Xin Li <xin3.li@...el.com>, Jason
 Gunthorpe <jgg@...pe.ca>, Tina Zhang <tina.zhang@...el.com>, Uros Bizjak
 <ubizjak@...il.com>, "Kirill A. Shutemov"
 <kirill.shutemov@...ux.intel.com>, linux-kernel@...r.kernel.org,
 kasan-dev@...glegroups.com, linux-hardening@...r.kernel.org,
 llvm@...ts.linux.dev
Subject: Re: [PATCH v2] x86/traps: Enable UBSAN traps on x86

On Mon, Jun 17 2024 at 16:06, Kees Cook wrote:
> On Tue, Jun 18, 2024 at 12:13:27AM +0200, Thomas Gleixner wrote:
>> In fact is_valid_bugaddr() should be globally fixed up to return bool to
>> match what the function name suggests.
>> 
>> The UD type information is x86 specific and has zero business in a
>> generic architecture agnostic function return value.
>> 
>> It's a sad state of affairs that I have to explain this to people who
>> care about code correctness. Readability and consistency are substantial
>> parts of correctness, really.
>
> Well, it's trade-offs. If get_ud_type() is in is_valid_bugaddr(), we
> have to call it _again_ outside of is_valid_bugaddr(). That's suboptimal
> as well. I was trying to find a reasonable way to avoid refactoring all
> architectures and to avoid code code.

There is not much of a trade-off. This is not the context switch hot
path, right?

Aside of that what is wrong with refactoring? If something does not fit
or the name does not make sense anymore then refactoring is the right
thing to do, no? It's not rocket science and just a little bit more work
but benefitial at the end.

> Looking at it all again, I actually think arch/x86/kernel/traps.c
> shouldn't call is_valid_bugaddr() at all. That usage can continue to
> stay in lib/bug.c, which is only ever used by x86 during very early
> boot, according to the comments in early_fixup_exception(). So just a
> direct replacement of is_valid_bugaddr() with the proposed get_ud_type()
> should be fine in arch/x86/kernel/traps.c.

I haven't looked at the details, but if that's the case then there is
even less of a reason to abuse is_valid_bugaddr().

That said it would still be sensible to convert is_valid_bugaddr() to a
boolean return value :)

Thanks,

        tglx