Re: [PATCH] driver core: faux: fix Undefined Behavior in faux_device_destroy()

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Jun 13, 2025 at 07:15:56PM +0000, marc.herbert@linux.intel.com wrote:
> From: Marc Herbert <marc.herbert@linux.intel.com>
> 
> Fixes undefined behavior that was spotted by Jonathan Cameron in
> https://lore.kernel.org/linux-cxl/20250609170509.00003625@huawei.com/
> 
> The possible consequences of the undefined behavior fixed here are fairly
> well documented across the Internet but to save research time and avoid
> doubts, I include a very short and simple demo below. I imagine kernel
> compilation flags and various other conditions may not make the
> consequences as bad as this example, however those conditions could change
> and this type of code is still Undefined Behavior no matter what.
> One of the best articles - there are many others:
> https://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html
> 
> Since commit b5ec6fd286dfa4 ("kbuild: Drop -Wdeclaration-after-statement"),
> it's now possible to use C99 declarations; the kernel is not constrained
> anymore to group all declarations at the top of a block like single-pass
> compilers used to require. This allows combining declarations and
> definitions in one place - like literally every other language and project
> does - and trivially fix undefined behavior like this.  This also reduces
> variable scope and avoids misuse between declaration and definition like
> uninitialized reads or writing to the wrong variable by mistake. C99
> declarations also allow using a lot more `const` (the default in some
> languages) which avoids some misuse after legitimate use.
> tl;dr: C99 declarations are not just a "codestyle" or "taste" issue;
> they are an important (and not mandatory) feature.
> 
> cc --version
>   cc (GCC) 15.1.1 20250425
> 
> for i in 0 1 2 g; do printf "gcc -O$i: "; gcc -O$i nullptrUB.c &&
>    ./a.out; done
> 
> gcc -O0: Segmentation fault (core dumped)
> gcc -O1: ptr is zero
> gcc -O2: ptr is NOT zero!!!
> gcc -O3: ptr is NOT zero!!!
> gcc -Og: ptr is zero
> 
> clang --version
>   clang version 19.1.7
> 
> clang -O0: Segmentation fault (core dumped)
> clang -O1: ptr is NOT zero!!!
> clang -O2: ptr is NOT zero!!!
> clang -O3: ptr is NOT zero!!!
> clang -Og: ptr is NOT zero!!!
> 
> int faux_device_destroy(int *ptr)
> {
>   int i = *ptr;  i++;
> 
>   // Because we dereferenced ptr, the compiler knows the pointer cannot
>   // be null (even when it is!) and can optimize this away.
>   if (!ptr) {
>     printf("ptr is zero\n");
>     return 0;
>   }
> 
>   printf("ptr is NOT zero!!!\n");
>   return 1;
> }
> 
> int main()
> {
>   struct timespec t1, t2;
>   clock_gettime(CLOCK_MONOTONIC, &t1);
>   clock_gettime(CLOCK_MONOTONIC, &t2);
> 
>   // Use the clock to hide zero from the compiler
>   int * zeroptr = (int *)(t2.tv_sec - t1.tv_sec);
> 
>   return faux_device_destroy(zeroptr);
> }
> 
> Fixes: 35fa2d88ca94 ("driver core: add a faux bus for use when a simple device/bus is needed")
> Signed-off-by: Marc Herbert <marc.herbert@linux.intel.com>

Great writeup, but as Miguel says, this isn't needed at all, the kernel
relies on the compiler to be sane :)

thanks,

greg k-h