Mathieu Malaterre [off-list ref] writes:

On Fri, Feb 15, 2019 at 7:14 AM Michael Ellerman [off-list ref] wrote:

quoted

GCC 8 warns about the logic in vr_get/set(), which with -Werror breaks
the build:

  In function ‘user_regset_copyin’,
      inlined from ‘vr_set’ at arch/powerpc/kernel/ptrace.c:628:9:
  include/linux/regset.h:295:4: error: ‘memcpy’ offset [-527, -529] is
  out of the bounds [0, 16] of object ‘vrsave’ with type ‘union
  <anonymous>’ [-Werror=array-bounds]
  arch/powerpc/kernel/ptrace.c: In function ‘vr_set’:
  arch/powerpc/kernel/ptrace.c:623:5: note: ‘vrsave’ declared here
     } vrsave;

This has been identified as a regression in GCC, see GCC bug 88273.

Good point, this does not seems this will be backported.

quoted

However we can avoid the warning and also simplify the logic and make
it more robust.

Currently we pass -1 as end_pos to user_regset_copyout(). This says
"copy up to the end of the regset".

The definition of the regset is:
        [REGSET_VMX] = {
                .core_note_type = NT_PPC_VMX, .n = 34,
                .size = sizeof(vector128), .align = sizeof(vector128),
                .active = vr_active, .get = vr_get, .set = vr_set
        },

The end is calculated as (n * size), ie. 34 * sizeof(vector128).

In vr_get/set() we pass start_pos as 33 * sizeof(vector128), meaning
we can copy up to sizeof(vector128) into/out-of vrsave.

The on-stack vrsave is defined as:
  union {
          elf_vrreg_t reg;
          u32 word;
  } vrsave;

And elf_vrreg_t is:
  typedef __vector128 elf_vrreg_t;

So there is no bug, but we rely on all those sizes lining up,
otherwise we would have a kernel stack exposure/overwrite on our
hands.

Rather than relying on that we can pass an explict end_pos based on
the sizeof(vrsave). The result should be exactly the same but it's
more obviously not over-reading/writing the stack and it avoids the
compiler warning.

maybe:

Link: https://lkml.org/lkml/2018/8/16/117

Hmm, I prefer not to include links because they're unlikely to last
forever like the git history.

If we do include them the preferred form is a link to lkml.kernel.org
using the message id. That way the message is recorded and also the link
is "guaranteed" to work in future, eg:

http://lkml.kernel.org/r/alpine.LRH.2.21.1808161041350.16413@math.ut.ee

In this case I don't think the link adds much over what I have in the
change log, in particular I did credit Meelis as the reporter.

In any case the warning is now gone:

Tested-by: Mathieu Malaterre <redacted>

Thanks.

cheers

quoted

Reported-by: Meelis Roos <redacted>
Reported-by: Mathieu Malaterre <redacted>
Cc: stable@vger.kernel.org
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>