All of lore.kernel.org
 help / color / mirror / Atom feed
* arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.
@ 2023-04-03  6:28 Dan Carpenter
  2023-04-03  8:03 ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2023-04-03  6:28 UTC (permalink / raw)
  To: oe-kbuild, Ard Biesheuvel
  Cc: lkp, oe-kbuild-all, linux-kernel, Catalin Marinas, Arnd Bergmann

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   00c7b5f4ddc5b346df62b757ec73f9357bb452af
commit: 3fc24ef32d3b9368f4c103dcd21d6a3f959b4870 arm64: compat: Implement misalignment fixups for multiword loads
config: arm64-randconfig-m041-20230329 (https://download.01.org/0day-ci/archive/20230402/202304021214.gekJ8yRc-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202304021214.gekJ8yRc-lkp@intel.com/

smatch warnings:
arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.

vim +/tinst2 +333 arch/arm64/kernel/compat_alignment.c

3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  310  int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs)
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  311  {
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  312  	union offset_union offset;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  313  	unsigned long instrptr;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  314  	int (*handler)(unsigned long addr, u32 instr, struct pt_regs *regs);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  315  	unsigned int type;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  316  	u32 instr = 0;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  317  	u16 tinstr = 0;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  318  	int isize = 4;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  319  	int thumb2_32b = 0;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  320  	int fault;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  321  
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  322  	instrptr = instruction_pointer(regs);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  323  
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  324  	if (compat_thumb_mode(regs)) {
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  325  		__le16 __user *ptr = (__le16 __user *)(instrptr & ~1);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  326  
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  327  		fault = alignment_get_thumb(regs, ptr, &tinstr);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  328  		if (!fault) {
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  329  			if (IS_T32(tinstr)) {
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  330  				/* Thumb-2 32-bit */
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  331  				u16 tinst2;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  332  				fault = alignment_get_thumb(regs, ptr + 1, &tinst2);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01 @333  				instr = ((u32)tinstr << 16) | tinst2;

Smatch is complaining that there is no error checking to see if the
copy_from_user() fails in alignment_get_thumb.  Eventually the syzbot
will learn to detect this as well.

Most distro kernels are going to automatically zero out stack variables
like tinst2 to prevent undefined behavior.

Presumably this is a fast path.  So setting "u16 tinst2 = 0;" does not
affect runtime speed for distro kernels and it might be the best
solution.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.
  2023-04-03  6:28 arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2' Dan Carpenter
@ 2023-04-03  8:03 ` Ard Biesheuvel
  2023-04-03  8:34   ` Dan Carpenter
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2023-04-03  8:03 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Catalin Marinas,
	Arnd Bergmann

On Mon, 3 Apr 2023 at 08:29, Dan Carpenter <error27@gmail.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   00c7b5f4ddc5b346df62b757ec73f9357bb452af
> commit: 3fc24ef32d3b9368f4c103dcd21d6a3f959b4870 arm64: compat: Implement misalignment fixups for multiword loads
> config: arm64-randconfig-m041-20230329 (https://download.01.org/0day-ci/archive/20230402/202304021214.gekJ8yRc-lkp@intel.com/config)
> compiler: aarch64-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202304021214.gekJ8yRc-lkp@intel.com/
>
> smatch warnings:
> arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.
>
> vim +/tinst2 +333 arch/arm64/kernel/compat_alignment.c
>
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  310  int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs)
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  311  {
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  312   union offset_union offset;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  313   unsigned long instrptr;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  314   int (*handler)(unsigned long addr, u32 instr, struct pt_regs *regs);
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  315   unsigned int type;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  316   u32 instr = 0;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  317   u16 tinstr = 0;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  318   int isize = 4;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  319   int thumb2_32b = 0;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  320   int fault;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  321
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  322   instrptr = instruction_pointer(regs);
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  323
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  324   if (compat_thumb_mode(regs)) {
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  325           __le16 __user *ptr = (__le16 __user *)(instrptr & ~1);
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  326
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  327           fault = alignment_get_thumb(regs, ptr, &tinstr);
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  328           if (!fault) {
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  329                   if (IS_T32(tinstr)) {
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  330                           /* Thumb-2 32-bit */
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  331                           u16 tinst2;
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  332                           fault = alignment_get_thumb(regs, ptr + 1, &tinst2);
> 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01 @333                           instr = ((u32)tinstr << 16) | tinst2;
>
> Smatch is complaining that there is no error checking to see if the
> copy_from_user() fails in alignment_get_thumb.  Eventually the syzbot
> will learn to detect this as well.
>

That shouldn't matter.

        u16 instr = 0;
        int fault;

        if (user_mode(regs))
                fault = get_user(instr, ip);
        else
                fault = get_kernel_nofault(instr, ip);

        *inst = __mem_to_opcode_thumb16(instr);

So the *inst assignment is never ambiguous here, regardless of whether
get_user() fails. The value could be wrong (i.e., get_user may have
failed after reading only one byte) but the value is never
uninitialized. Then, the fault value is always propagated so the
calling function will not succeed spuriously when this happens.

> Most distro kernels are going to automatically zero out stack variables
> like tinst2 to prevent undefined behavior.
>

instr is already zeroed out.

> Presumably this is a fast path.  So setting "u16 tinst2 = 0;" does not
> affect runtime speed for distro kernels and it might be the best
> solution.
>

Performance is not an issue here - this is a misalignment fixup
handler, which we copied from the 32-bit ARM tree only for compat
reasons, but anyone who cares about performance would not use
misaligned accesses or even run 32-bit code on a 64-bit system.

However, given that this code originates in the arch/arm tree, I am
reluctant make such 'correctness' tweaks while the logic is sound and
unambiguous.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.
  2023-04-03  8:03 ` Ard Biesheuvel
@ 2023-04-03  8:34   ` Dan Carpenter
  2023-04-03  9:10     ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2023-04-03  8:34 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Catalin Marinas,
	Arnd Bergmann

On Mon, Apr 03, 2023 at 10:03:15AM +0200, Ard Biesheuvel wrote:
> On Mon, 3 Apr 2023 at 08:29, Dan Carpenter <error27@gmail.com> wrote:
> >
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   00c7b5f4ddc5b346df62b757ec73f9357bb452af
> > commit: 3fc24ef32d3b9368f4c103dcd21d6a3f959b4870 arm64: compat: Implement misalignment fixups for multiword loads
> > config: arm64-randconfig-m041-20230329 (https://download.01.org/0day-ci/archive/20230402/202304021214.gekJ8yRc-lkp@intel.com/config)
> > compiler: aarch64-linux-gcc (GCC) 12.1.0
> >
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <error27@gmail.com>
> > | Link: https://lore.kernel.org/r/202304021214.gekJ8yRc-lkp@intel.com/
> >
> > smatch warnings:
> > arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.
> >
> > vim +/tinst2 +333 arch/arm64/kernel/compat_alignment.c
> >
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  310  int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs)
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  311  {
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  312   union offset_union offset;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  313   unsigned long instrptr;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  314   int (*handler)(unsigned long addr, u32 instr, struct pt_regs *regs);
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  315   unsigned int type;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  316   u32 instr = 0;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  317   u16 tinstr = 0;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  318   int isize = 4;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  319   int thumb2_32b = 0;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  320   int fault;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  321
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  322   instrptr = instruction_pointer(regs);
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  323
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  324   if (compat_thumb_mode(regs)) {
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  325           __le16 __user *ptr = (__le16 __user *)(instrptr & ~1);
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  326
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  327           fault = alignment_get_thumb(regs, ptr, &tinstr);
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  328           if (!fault) {
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  329                   if (IS_T32(tinstr)) {
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  330                           /* Thumb-2 32-bit */
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  331                           u16 tinst2;
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  332                           fault = alignment_get_thumb(regs, ptr + 1, &tinst2);
> > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01 @333                           instr = ((u32)tinstr << 16) | tinst2;
> >
> > Smatch is complaining that there is no error checking to see if the
> > copy_from_user() fails in alignment_get_thumb.  Eventually the syzbot
> > will learn to detect this as well.
> >
> 
> That shouldn't matter.
> 
>         u16 instr = 0;
>         int fault;
> 
>         if (user_mode(regs))
>                 fault = get_user(instr, ip);
>         else
>                 fault = get_kernel_nofault(instr, ip);
> 
>         *inst = __mem_to_opcode_thumb16(instr);
> 
> So the *inst assignment is never ambiguous here, regardless of whether
> get_user() fails. The value could be wrong (i.e., get_user may have
> failed after reading only one byte) but the value is never
> uninitialized. Then, the fault value is always propagated so the
> calling function will not succeed spuriously when this happens.
> 

I don't know what code that is...  We are looking at different code.
For me on linux-next it looks like this:

arch/arm64/kernel/compat_alignment.c
   297  static int alignment_get_thumb(struct pt_regs *regs, __le16 __user *ip, u16 *inst)
   298  {
   299          __le16 instr = 0;
   300          int fault;
   301  
   302          fault = get_user(instr, ip);
   303          if (fault)
   304                  return fault;

*inst not initialized.

   305  
   306          *inst = __le16_to_cpu(instr);
   307          return 0;
   308  }
   309  
   310  int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs)
   311  {
   312          union offset_union offset;
   313          unsigned long instrptr;
   314          int (*handler)(unsigned long addr, u32 instr, struct pt_regs *regs);
   315          unsigned int type;
   316          u32 instr = 0;
   317          u16 tinstr = 0;
   318          int isize = 4;
   319          int thumb2_32b = 0;
   320          int fault;
   321  
   322          instrptr = instruction_pointer(regs);
   323  
   324          if (compat_thumb_mode(regs)) {
   325                  __le16 __user *ptr = (__le16 __user *)(instrptr & ~1);
   326  
   327                  fault = alignment_get_thumb(regs, ptr, &tinstr);
   328                  if (!fault) {
   329                          if (IS_T32(tinstr)) {
   330                                  /* Thumb-2 32-bit */
   331                                  u16 tinst2;
   332                                  fault = alignment_get_thumb(regs, ptr + 1, &tinst2);
   333                                  instr = ((u32)tinstr << 16) | tinst2;
                                                                      ^^^^^^
Uninitialized variable here.

   334                                  thumb2_32b = 1;
   335                          } else {
   336                                  isize = 2;
   337                                  instr = thumb2arm(tinstr);
   338                          }
   339                  }
   340          } else {

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.
  2023-04-03  8:34   ` Dan Carpenter
@ 2023-04-03  9:10     ` Ard Biesheuvel
  0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2023-04-03  9:10 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Catalin Marinas,
	Arnd Bergmann

On Mon, 3 Apr 2023 at 10:34, Dan Carpenter <error27@gmail.com> wrote:
>
> On Mon, Apr 03, 2023 at 10:03:15AM +0200, Ard Biesheuvel wrote:
> > On Mon, 3 Apr 2023 at 08:29, Dan Carpenter <error27@gmail.com> wrote:
> > >
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > head:   00c7b5f4ddc5b346df62b757ec73f9357bb452af
> > > commit: 3fc24ef32d3b9368f4c103dcd21d6a3f959b4870 arm64: compat: Implement misalignment fixups for multiword loads
> > > config: arm64-randconfig-m041-20230329 (https://download.01.org/0day-ci/archive/20230402/202304021214.gekJ8yRc-lkp@intel.com/config)
> > > compiler: aarch64-linux-gcc (GCC) 12.1.0
> > >
> > > If you fix the issue, kindly add following tag where applicable
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Reported-by: Dan Carpenter <error27@gmail.com>
> > > | Link: https://lore.kernel.org/r/202304021214.gekJ8yRc-lkp@intel.com/
> > >
> > > smatch warnings:
> > > arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.
> > >
> > > vim +/tinst2 +333 arch/arm64/kernel/compat_alignment.c
> > >
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  310  int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs)
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  311  {
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  312   union offset_union offset;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  313   unsigned long instrptr;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  314   int (*handler)(unsigned long addr, u32 instr, struct pt_regs *regs);
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  315   unsigned int type;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  316   u32 instr = 0;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  317   u16 tinstr = 0;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  318   int isize = 4;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  319   int thumb2_32b = 0;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  320   int fault;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  321
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  322   instrptr = instruction_pointer(regs);
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  323
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  324   if (compat_thumb_mode(regs)) {
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  325           __le16 __user *ptr = (__le16 __user *)(instrptr & ~1);
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  326
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  327           fault = alignment_get_thumb(regs, ptr, &tinstr);
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  328           if (!fault) {
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  329                   if (IS_T32(tinstr)) {
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  330                           /* Thumb-2 32-bit */
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  331                           u16 tinst2;
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  332                           fault = alignment_get_thumb(regs, ptr + 1, &tinst2);
> > > 3fc24ef32d3b93 Ard Biesheuvel 2022-07-01 @333                           instr = ((u32)tinstr << 16) | tinst2;
> > >
> > > Smatch is complaining that there is no error checking to see if the
> > > copy_from_user() fails in alignment_get_thumb.  Eventually the syzbot
> > > will learn to detect this as well.
> > >
> >
> > That shouldn't matter.
> >
> >         u16 instr = 0;
> >         int fault;
> >
> >         if (user_mode(regs))
> >                 fault = get_user(instr, ip);
> >         else
> >                 fault = get_kernel_nofault(instr, ip);
> >
> >         *inst = __mem_to_opcode_thumb16(instr);
> >
> > So the *inst assignment is never ambiguous here, regardless of whether
> > get_user() fails. The value could be wrong (i.e., get_user may have
> > failed after reading only one byte) but the value is never
> > uninitialized. Then, the fault value is always propagated so the
> > calling function will not succeed spuriously when this happens.
> >
>
> I don't know what code that is...  We are looking at different code.
> For me on linux-next it looks like this:
>

My bad (and this illustrates my point about not deviating from the
original when there is a need for two copies of the code to exist in
the same tree, only not in the way I thought)

So the ARM code is correct, and the arm64 version deviates from it,
and introduces the issue you are reporting.

I agree that initializing tinst2 to 0 is the appropriate course of action here.

Thanks,
Ard.


> arch/arm64/kernel/compat_alignment.c
>    297  static int alignment_get_thumb(struct pt_regs *regs, __le16 __user *ip, u16 *inst)
>    298  {
>    299          __le16 instr = 0;
>    300          int fault;
>    301
>    302          fault = get_user(instr, ip);
>    303          if (fault)
>    304                  return fault;
>
> *inst not initialized.
>
>    305
>    306          *inst = __le16_to_cpu(instr);
>    307          return 0;
>    308  }
>    309
>    310  int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs)
>    311  {
>    312          union offset_union offset;
>    313          unsigned long instrptr;
>    314          int (*handler)(unsigned long addr, u32 instr, struct pt_regs *regs);
>    315          unsigned int type;
>    316          u32 instr = 0;
>    317          u16 tinstr = 0;
>    318          int isize = 4;
>    319          int thumb2_32b = 0;
>    320          int fault;
>    321
>    322          instrptr = instruction_pointer(regs);
>    323
>    324          if (compat_thumb_mode(regs)) {
>    325                  __le16 __user *ptr = (__le16 __user *)(instrptr & ~1);
>    326
>    327                  fault = alignment_get_thumb(regs, ptr, &tinstr);
>    328                  if (!fault) {
>    329                          if (IS_T32(tinstr)) {
>    330                                  /* Thumb-2 32-bit */
>    331                                  u16 tinst2;
>    332                                  fault = alignment_get_thumb(regs, ptr + 1, &tinst2);
>    333                                  instr = ((u32)tinstr << 16) | tinst2;
>                                                                       ^^^^^^
> Uninitialized variable here.
>
>    334                                  thumb2_32b = 1;
>    335                          } else {
>    336                                  isize = 2;
>    337                                  instr = thumb2arm(tinstr);
>    338                          }
>    339                  }
>    340          } else {
>
> regards,
> dan carpenter
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.
@ 2023-04-02  5:05 kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-04-02  5:05 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
CC: linux-kernel@vger.kernel.org
TO: Ard Biesheuvel <ardb@kernel.org>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Arnd Bergmann <arnd@arndb.de>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   00c7b5f4ddc5b346df62b757ec73f9357bb452af
commit: 3fc24ef32d3b9368f4c103dcd21d6a3f959b4870 arm64: compat: Implement misalignment fixups for multiword loads
date:   7 months ago
:::::: branch date: 8 hours ago
:::::: commit date: 7 months ago
config: arm64-randconfig-m041-20230329 (https://download.01.org/0day-ci/archive/20230402/202304021214.gekJ8yRc-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202304021214.gekJ8yRc-lkp@intel.com/

smatch warnings:
arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2'.

vim +/tinst2 +333 arch/arm64/kernel/compat_alignment.c

3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  309  
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  310  int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs)
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  311  {
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  312  	union offset_union offset;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  313  	unsigned long instrptr;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  314  	int (*handler)(unsigned long addr, u32 instr, struct pt_regs *regs);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  315  	unsigned int type;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  316  	u32 instr = 0;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  317  	u16 tinstr = 0;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  318  	int isize = 4;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  319  	int thumb2_32b = 0;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  320  	int fault;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  321  
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  322  	instrptr = instruction_pointer(regs);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  323  
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  324  	if (compat_thumb_mode(regs)) {
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  325  		__le16 __user *ptr = (__le16 __user *)(instrptr & ~1);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  326  
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  327  		fault = alignment_get_thumb(regs, ptr, &tinstr);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  328  		if (!fault) {
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  329  			if (IS_T32(tinstr)) {
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  330  				/* Thumb-2 32-bit */
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  331  				u16 tinst2;
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01  332  				fault = alignment_get_thumb(regs, ptr + 1, &tinst2);
3fc24ef32d3b93 Ard Biesheuvel 2022-07-01 @333  				instr = ((u32)tinstr << 16) | tinst2;

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-03  9:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-03  6:28 arch/arm64/kernel/compat_alignment.c:333 do_compat_alignment_fixup() error: uninitialized symbol 'tinst2' Dan Carpenter
2023-04-03  8:03 ` Ard Biesheuvel
2023-04-03  8:34   ` Dan Carpenter
2023-04-03  9:10     ` Ard Biesheuvel
  -- strict thread matches above, loose matches on Subject: below --
2023-04-02  5:05 kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.