Skip to content

Commit

Permalink
powerpc/lib: Avoid array bounds warnings in vec ops
Browse files Browse the repository at this point in the history
I'M TEMPORARILY CARRYING THIS PATCH IN MY -next TREE, SO
-Wstringop-overflow CAN SHOW UP IN linux-next ASAP.

ONCE MICHAEL ADD IT TO HIS -next TREE I WILL REMOVE IT
FROM MINE. :)

Building with GCC 13 (which has -array-bounds enabled) there are several
warnings in sstep.c along the lines of:

  In function ‘do_byte_reverse’,
      inlined from ‘do_vec_load’ at arch/powerpc/lib/sstep.c:691:3,
      inlined from ‘emulate_loadstore’ at arch/powerpc/lib/sstep.c:3439:9:
  arch/powerpc/lib/sstep.c:289:23: error: array subscript 2 is outside array bounds of ‘u8[16]’ {aka ‘unsigned char[16]’} [-Werror=array-bounds=]
    289 |                 up[2] = byterev_8(up[1]);
        |                 ~~~~~~^~~~~~~~~~~~~~~~~~
  arch/powerpc/lib/sstep.c: In function ‘emulate_loadstore’:
  arch/powerpc/lib/sstep.c:681:11: note: at offset 16 into object ‘u’ of size 16
    681 |         } u = {};
        |           ^

do_byte_reverse() supports a size up to 32 bytes, but in these cases the
caller is only passing a 16 byte buffer. In practice there is no bug,
do_vec_load() is only called from the LOAD_VMX case in emulate_loadstore().
That in turn is only reached when analyse_instr() recognises VMX ops,
and in all cases the size is no greater than 16:

  $ git grep -w LOAD_VMX arch/powerpc/lib/sstep.c
  arch/powerpc/lib/sstep.c:                        op->type = MKOP(LOAD_VMX, 0, 1);
  arch/powerpc/lib/sstep.c:                        op->type = MKOP(LOAD_VMX, 0, 2);
  arch/powerpc/lib/sstep.c:                        op->type = MKOP(LOAD_VMX, 0, 4);
  arch/powerpc/lib/sstep.c:                        op->type = MKOP(LOAD_VMX, 0, 16);

Similarly for do_vec_store().

Although the warning is incorrect, the code would be safer if it clamped
the size from the caller to the known size of the buffer. Do that using
min_t().

Reported-by: Bagas Sanjaya <[email protected]>
Reported-by: Jan-Benedict Glaw <[email protected]>
Reported-by: Gustavo A. R. Silva <[email protected]>
Reviewed-by: Gustavo A. R. Silva <[email protected]>
Build-tested-by: Gustavo A. R. Silva <[email protected]>
Signed-off-by: Michael Ellerman <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
  • Loading branch information
GustavoARSilva authored and intel-lab-lkp committed Nov 23, 2023
1 parent 275f511 commit 3669e15
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions arch/powerpc/lib/sstep.c
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ static nokprobe_inline int do_vec_load(int rn, unsigned long ea,
if (err)
return err;
if (unlikely(cross_endian))
do_byte_reverse(&u.b[ea & 0xf], size);
do_byte_reverse(&u.b[ea & 0xf], min_t(size_t, size, sizeof(u)));
preempt_disable();
if (regs->msr & MSR_VEC)
put_vr(rn, &u.v);
Expand Down Expand Up @@ -719,7 +719,7 @@ static nokprobe_inline int do_vec_store(int rn, unsigned long ea,
u.v = current->thread.vr_state.vr[rn];
preempt_enable();
if (unlikely(cross_endian))
do_byte_reverse(&u.b[ea & 0xf], size);
do_byte_reverse(&u.b[ea & 0xf], min_t(size_t, size, sizeof(u)));
return copy_mem_out(&u.b[ea & 0xf], ea, size, regs);
}
#endif /* CONFIG_ALTIVEC */
Expand Down

0 comments on commit 3669e15

Please sign in to comment.