linux-kernel - Re: [PATCH v4 07/14] arm64: Add support for suppressing warning backtraces

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]

Message-ID: <CAGegRW5r3V2-_44-X353vS-GZwDYG=SVwc6MzSGE8GdFQuFoKA@mail.gmail.com>
Date: Thu, 13 Mar 2025 17:40:59 +0100
From: Alessandro Carminati <acarmina@...hat.com>
To: Will Deacon <will@...nel.org>
Cc: linux-kselftest@...r.kernel.org, David Airlie <airlied@...il.com>, 
	Arnd Bergmann <arnd@...db.de>, Maíra Canal <mcanal@...lia.com>, 
	Dan Carpenter <dan.carpenter@...aro.org>, Kees Cook <keescook@...omium.org>, 
	Daniel Diaz <daniel.diaz@...aro.org>, David Gow <davidgow@...gle.com>, 
	Arthur Grillo <arthurgrillo@...eup.net>, Brendan Higgins <brendan.higgins@...ux.dev>, 
	Naresh Kamboju <naresh.kamboju@...aro.org>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Maxime Ripard <mripard@...nel.org>, 
	Ville Syrjälä <ville.syrjala@...ux.intel.com>, 
	Daniel Vetter <daniel@...ll.ch>, Thomas Zimmermann <tzimmermann@...e.de>, 
	Guenter Roeck <linux@...ck-us.net>, Alessandro Carminati <alessandro.carminati@...il.com>, 
	Jani Nikula <jani.nikula@...el.com>, dri-devel@...ts.freedesktop.org, 
	kunit-dev@...glegroups.com, linux-arch@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-parisc@...r.kernel.org, 
	linuxppc-dev@...ts.ozlabs.org, linux-riscv@...ts.infradead.org, 
	linux-s390@...r.kernel.org, linux-sh@...r.kernel.org, 
	loongarch@...ts.linux.dev, x86@...nel.org, 
	Linux Kernel Functional Testing <lkft@...aro.org>, Catalin Marinas <catalin.marinas@....com>
Subject: Re: [PATCH v4 07/14] arm64: Add support for suppressing warning backtraces

Hello Will,

On Thu, Mar 13, 2025 at 1:25 PM Will Deacon <will@...nel.org> wrote:
>
> On Thu, Mar 13, 2025 at 11:43:22AM +0000, Alessandro Carminati wrote:
> > diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
> > index 28be048db3f6..044c5e24a17d 100644
> > --- a/arch/arm64/include/asm/bug.h
> > +++ b/arch/arm64/include/asm/bug.h
> > @@ -11,8 +11,14 @@
> >
> >  #include <asm/asm-bug.h>
> >
> > +#ifdef HAVE_BUG_FUNCTION
> > +# define __BUG_FUNC  __func__
> > +#else
> > +# define __BUG_FUNC  NULL
> > +#endif
> > +
> >  #define __BUG_FLAGS(flags)                           \
> > -     asm volatile (__stringify(ASM_BUG_FLAGS(flags)));
> > +     asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i" (__BUG_FUNC));
>
> Why is 'i' the right asm constraint to use here? It seems a bit odd to
> use that for a pointer.

I received this code as legacy from a previous version.
In my review, I considered the case when HAVE_BUG_FUNCTION is defined:
Here, __BUG_FUNC is defined as __func__, which is the name of the
current function as a string literal.
Using the constraint "i" seems appropriate to me in this case.

However, when HAVE_BUG_FUNCTION is not defined:
__BUG_FUNC is defined as NULL. Initially, I considered it literal 0,
but after investigating your concern, I found:

```
$ echo -E "#include <stdio.h>\n#include <stddef.h>\nint main()
{\nreturn 0;\n}" | aarch64-linux-gnu-gcc -E -dM - | grep NULL
#define NULL ((void *)0)
```

I realized that NULL is actually a pointer that is not a link time
symbol, and using the "i" constraint with NULL may result in undefined
behavior.

Would the following alternative definition for __BUG_FUNC be more convincing?

```
#ifdef HAVE_BUG_FUNCTION
    #define __BUG_FUNC __func__
#else
    #define __BUG_FUNC (uintptr_t)0
#endif
```
Let me know your thoughts.

>
> Will
>



--
---
172