On 3/6/26 05:17, Peter Maydell wrote:
Our definition of the target_fpstate_32 struct doesn't match the
kernel's version.  We only use this struct definition in the
definition of 'struct sigframe', where it is used in a field that is
present only for legacy reasons to retain the offset of the following
'extramask' field.  So really all that matters is its length, and we
do get that right; but our previous definition using
X86LegacySaveArea implicitly added an extra alignment constraint
(because X86LegacySaveArea is tagged as 16-aligned) which the real
target_fpstate_32 does not have.  Because we allocate and use a
'struct sigframe' on the guest's stack with the guest's alignment
requirements, this resulted in the undefined-behaviour sanitizer
complaining during 'make check-tcg' for i386-linux-user:

../../linux-user/i386/signal.c:471:35: runtime error: member access within 
misaligned address 0x1000c07f75ec for type 'struct sigframe', which requires 16 
byte alignment
0x1000c07f75ec: note: pointer points here
   00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  
00 00 00 00 00 00 00 00
               ^

../../linux-user/i386/signal.c:808:5: runtime error: member access within 
misaligned address 0x1000c07f75f4 for type 'struct target_sigcontext_32', which 
requires 8 byte alignment
0x1000c07f75f4: note: pointer points here
   0a 00 00 00 33 00 00 00  00 00 00 00 2b 00 00 00  2b 00 00 00 40 05 80 40  
f4 7f 10 08 58 05 80 40
               ^

and various similar errors.

Replace the use of X86LegacyXSaveArea with a set of fields that match
the kernel _fpstate_32 struct, and assert that the length is correct.
We could equally have used
    uint8_t legacy_area[512];
but following the kernel is probably less confusing overall.

Since in target/i386/cpu.h we assert that X86LegacySaveArea is 512
bytes, and in linux-user/i386/signal.c we assert that
target_fregs_state is (32 + 80) bytes, the new assertion confirms
that we didn't change the size of target_fpstate_32 here, only its
alignment requirements.

Cc: [email protected]
Signed-off-by: Peter Maydell <[email protected]>
---
  linux-user/i386/signal.c | 25 ++++++++++++++++++++++++-
  1 file changed, 24 insertions(+), 1 deletion(-)

I think it's more complicated than that. I think this is indicative of the alignment being wrong. In fact, I have a vague memory that we have an Issue outstanding for this, which I looked into once upon a time, and I think we're well out of spec.

There are two parts: fpregs, which should be 64-byte aligned (for xsave/xrestore), and the main signal frame, which must be x % 16 == 16 - sizeof(void *), so that SP, after pushing the return address, is 16-byte aligned.

I can't immediately find the issue.  I'll see about writing a test case.


r~

Reply via email to