linux-kernel - Re: [PATCH v3 1/3] vdso: Switch get/put unaligned from packed struct to memcpy

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: <CAP-5=fWbmo3ejmeWbweSk5waPtS2VTc1obtaWiibZC3cVmvVvg@mail.gmail.com>
Date: Tue, 22 Jul 2025 09:44:11 -0700
From: Ian Rogers <irogers@...gle.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: kernel test robot <lkp@...el.com>, Eric Biggers <ebiggers@...gle.com>, Yuzhuo Jing <yuzhuo@...gle.com>, 
	Andy Lutomirski <luto@...nel.org>, Vincenzo Frascino <vincenzo.frascino@....com>, 
	Arnaldo Carvalho de Melo <acme@...hat.com>, Al Viro <viro@...iv.linux.org.uk>, 
	Christophe Leroy <christophe.leroy@...roup.eu>, "Jason A. Donenfeld" <Jason@...c4.com>, 
	linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org, 
	llvm@...ts.linux.dev, oe-kbuild-all@...ts.linux.dev
Subject: Re: [PATCH v3 1/3] vdso: Switch get/put unaligned from packed struct
 to memcpy

On Tue, Jul 22, 2025 at 8:56 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> Ian!
>
> On Sat, Jul 05 2025 at 08:05, kernel test robot wrote:
> > kernel test robot noticed the following build warnings:
> >
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on tip/timers/vdso v6.16-rc4 next-20250704]
> > [cannot apply to acme/perf/core]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url:    https://github.com/intel-lab-lkp/linux/commits/Ian-Rogers/vdso-Switch-get-put-unaligned-from-packed-struct-to-memcpy/20250626-135005
> > base:   linus/master
> > patch link:    https://lore.kernel.org/r/20250626054826.433453-2-irogers%40google.com
> > patch subject: [PATCH v3 1/3] vdso: Switch get/put unaligned from packed struct to memcpy
> > config: s390-randconfig-002-20250705 (https://download.01.org/0day-ci/archive/20250705/202507050736.b4hX0Xks-lkp@intel.com/config)
> > compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 61529d9e36fa86782a2458e6bdeedf7f376ef4b5)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250705/202507050736.b4hX0Xks-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@...el.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202507050736.b4hX0Xks-lkp@intel.com/
> >
> > All warnings (new ones prefixed by >>):
> >
> >    In file included from arch/s390/boot/decompressor.c:48:
> >    In file included from arch/s390/include/uapi/../../../../lib/decompress_unlz4.c:10:
> >    In file included from arch/s390/include/uapi/../../../../lib/lz4/lz4_decompress.c:36:
> >>> arch/s390/include/uapi/../../../../lib/lz4/lz4defs.h:109:9: warning: default initialization of an object of type 'typeof (*((const U16 *)ptr))' (aka 'const unsigned short') leaves the object uninitialized [-Wdefault-const-init-var-unsafe]
> >      109 |         return get_unaligned((const U16 *)ptr);
> >          |                ^
>
> Any update on this one?

Hi Thomas!

Thanks for the interest. I'm not sure how to resolve this.
Basically the story in the code is:

  get_unaligned((const U16 *)ptr);

is being expanded by:

  #define get_unaligned(ptr)      __get_unaligned_t(typeof(*(ptr)), (ptr))
  #define __get_unaligned_t(type, ptr) ({
          \
       type __get_unaligned_ctrl_type __always_unused;                 \
       __unqual_scalar_typeof(__get_unaligned_ctrl_type) __get_unaligned_val; \
       __builtin_memcpy(&__get_unaligned_val, (void *)(ptr),           \
                        sizeof(__get_unaligned_val));                  \
       __get_unaligned_val;                                            \
  })

to:

  ({
  const U16 __get_unaligned_ctrl_type __always_unused;
  __unqual_scalar_typeof(__get_unaligned_ctrl_type) __get_unaligned_val;
  __builtin_memcpy(&__get_unaligned_val, (void *)(ptr),
   sizeof(__get_unaligned_val));
  __get_unaligned_val;
  })

which is then expanded to:

  ({
  const U16 __get_unaligned_ctrl_type __always_unused;
  U16 __get_unaligned_val;
  __builtin_memcpy(&__get_unaligned_val, (void *)(ptr),
   sizeof(__get_unaligned_val));
  __get_unaligned_val;
  })

so the analysis is correct that __get_unaligned_ctrl_type is
unused, however, I've also put __always_unused on the variable.
The purpose of that variable is for __unqual_scalar_typeof that
passes the value to _Generic. I truly don't care about that
variable but I care about it causing _Generic to get the correct
type for __get_unaligned_val (that value can't be const as it is
the destination of the memcpy). Why even declare
__get_unaligned_ctrl_type? Well we want the result of __get_unaligned_t to
match its type argument which needn't match the dereference of
its pointer argument (*ptr) - I accept that in many cases it
will. The value passed to _Generic is an expression and not a
type, so declaring the variable and using the variable for
_Generic allows this.

My feeling is that my patch is correct and I'm not clear how to
improve upon it. I believe it is a weakness in the analysis that
the __always_unused isn't having an effect.

Should adding new warnings for analysis on the code base be
allowed for this patch? Well the upside to the patch is getting
closer to not requiring -fno-strict-aliasing, or not introducing
requiring that flag for things in the tools directory. I think
the upside is good, I don't know how to mitigate the downside
with poor analysis by certain tools.

Thanks,
Ian