Re: kern/60539 (XSAVE changes break ucontext userspace API)

The following reply was made to PR kern/60539; it has been noted by GNATS.

From: Taylor R Campbell <[email protected]>
To: [email protected]
Cc: 
Subject: Re: kern/60539 (XSAVE changes break ucontext userspace API)
Date: Wed, 5 Aug 2026 15:36:03 +0000

 So, the good news is:
 
 1. There is a one-character change that should make this work on all
    versions of NetBSD, for programs that don't use AVX (which wouldn't
    work before, and which it looks like this program doesn't do
    anyway) or don't try to write back registers to be restored on
    signal handler return:
 
 -# define XMM_sig(p,i) (((struct fxsave64*)(p)->uc_mcontext.__fpregs)->fx_x=
 mm[i])
 +# define XMM_sig(p,i) (((struct fxsave64*)&(p)->uc_mcontext.__fpregs)->fx_=
 xmm[i])
 
    This change is needed because the mcontext_t member __fpregs
    changed from being an array to being a union:
 
 	-typedef char __fpregset_t[512] __aligned(8);
 	+typedef union {
 	+	char	__fxsave[512] __aligned(8);
 	+	struct {
 	...
 	+	}	__xsave;
 	+} __fpregset_t;
 
    And although the change I made to __fpregset_t isn't quite
    API-compatible (sorry), you just can take a pointer to the array
    itself with & (instead taking a pointer to the first element of the
    array, which is what you get without the &) and convert it to a
    pointer to the struct fxsave64 object.  So the change to XMM_sig
    will work on older versions of NetBSD just as well as >=3D11.
 
 2. Existing binaries that don't use AVX registers are unaffected by
    any ABI consequences of the XSAVE change, and it looks like that
    covers the code you quoted.
 
 The bad news -- other than the API breakage requiring randomly
 sprinkling ampersands like you're in a fight with the Rust borrow
 checker -- is that if the program _does_ use AVX, then writing back to
 the x87/SSE (st(i), xmmN) registers in the FXSAVE part of the
 ucontext_t will have no effect when the signal handler returns.
 
 Signal handlers can still _read_ x87/SSE registers out of this
 location -- it's just that _writing_ to them won't change the content
 that gets restored into the registers on signal handler return.
 
 The reason is that the kernel will lay out two separate copies of the
 FXSAVE area (x87/SSE state), which I did in order to make it easy to
 verify that the XSAVE patch (to save and restore registers _beyond_
 x87/SSE: AVX, AVX-512, AMX, whatever) only has an effect on processes
 that actually require the extra area, when they require it.
 
 The layout on the stack on signal delivery looks something like this,
 with higher addresses first:
 
          +-----------------------+ \
          | Hi16_ZMM              |  |
          | --------------------- |  |
          | ZMM_Hi256             |  |
          | --------------------- |  > XSAVE
          | YMM_Hi128             |  |
          | --------------------- |  |
          | FXSAVE in XSAVE       |  |
          +-----------------------+ /
          | (opt. padding bytes)  |
          +-----------------------+ \
          | FXSAVE in ucontext_t  |  |
          | --------------------- |  > ucontext_t
          | rest of ucontext_t    |  |
          +-----------------------+ /
          | siginfo_t             |
          +-----------------------+
          | return address        |
          +-----------------------+ <--- rsp on signal handler entry
 
 I considered making the XSAVE and ucontext_t areas overlap, like this:
 
        / +-----------------------+
       |  | Hi16_ZMM              |
       |  | --------------------- |
       |  | ZMM_Hi256             |
 XSAVE <  | --------------------- |
       |  | YMM_Hi128             |
       |  *-----------------------* \
       |  | FXSAVE in ucontext_t  |  |
        \ *-----------------------*  > ucontext_t
          | rest of ucontext_t    |  |
          +-----------------------+ /
          | siginfo_t             |
          +-----------------------+
          | return address        |
          +-----------------------+ <--- rsp on signal handler entry
 
 However, when the signal handler returns to the return address we
 provided, it enters the signal trampoline, which calls setcontext(2):
 
      72 NENTRY(__sigtramp_siginfo_2)
      73 	movq	%r15,%rdi
      74 	movq	$SYS_setcontext, %rax
      75 	syscall
 
 https://nxr.netbsd.org/xref/src/lib/libc/arch/x86_64/sys/__sigtramp2.S?r=3D=
 1.9
 
 And the first thing setcontext(2) does is copyin sizeof(ucontext_t)
 bytes -- that is, _only_ the ucontext_t, _not including_ the rest of
 the XSAVE area, because at this point the machine-independent
 setcontext(2) logic has no way to know that the downstream
 machine-dependent cpu_setmcontext logic is hungry for more bytes:
 
     341 int
     342 sys_setcontext(struct lwp *l, const struct sys_setcontext_args *uap,
     343     register_t *retval)
     344 {
     345 	/* {
     346 		syscallarg(const ucontext_t *) ucp;
     347 	} */
     348 	struct proc *p =3D l->l_proc;
     349 	ucontext_t uc;
     350 	int error;
     351=20
     352 	error =3D copyin(SCARG(uap, ucp), &uc, sizeof (uc));
 ...
     358 	error =3D setucontext(l, &uc);
 
 https://nxr.netbsd.org/xref/src/sys/kern/sys_sig.c?r=3D1.62#341
 
 And by the time control reaches cpu_setmcontext, it doesn't even know
 where it came from in userland, so it doesn't know where to copyin the
 rest of the XSAVE area from.  I could in principle have added more MD
 hooks to sys_setcontext to tell conditionally copy in larger chunks
 but that seemed like a very risky change to pull up to 11 in order to
 fix a long-term embarrassing x86-only bug.
 
 Instead, I just put the XSAVE area in a separate space on the stack
 (again, to keep the stack manipulation logic in sendsig_siginfo very
 easy to audit), and carved out 16 bytes of the 512-byte FXSAVE area
 that are architecturally unused (bytes 511:416 are not `reserved' like
 bytes 463:416 but `not use[d]' by the architecture according to the
 Intel manual, and `Reserved, IGN' rather than (e.g.) `Reserved, MBZ'
 according to the AMD manual), to store a pointer to and length of the
 XSAVE area.
 
 That way, the x86 cpu_setmcontext can find it to copy it in (after
 validating the length) in order to restore the state:
 
    2264 	if ((flags & _UC_XSAVE) !=3D 0) {
    2265 		const struct xsave_header *user_xsave =3D
    2266 		    (void *)(uintptr_t)mcp->__fpregs.__xsave.__xsaveptr;
    2267=20
    2268 		xsavelen =3D mcp->__fpregs.__xsave.__xsavelen;
    2269 		error =3D process_verify_xsavelen(l, xsavelen);
    2270 		if (error !=3D 0)
    2271 			goto out;
    2272 		xsavebuf =3D kmem_alloc(xsavelen, KM_SLEEP);
    2273 		error =3D copyin(user_xsave, xsavebuf, xsavelen);
    2274 		if (error !=3D 0)
    2275 			goto out;
    2276 		error =3D process_verify_xsave(l, xsavebuf, xsavelen);
    2277 		if (error !=3D 0)
    2278 			goto out;
    2279 	}
 
 https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/machdep.c?r=3D1.380#22=
 55
 
 (That's why I changed the mcontext_t __fpregs member to be a union: so
 I could add named __xsaveptr/__xsavelen members.  I don't know why it
 was ever an array, anyway; that didn't facilitate access to the CPU
 state the way that, say, a struct with st0/st1/.../st7/xmm0/xmm1/...
 members would have.)
 
 So, if you want access to state beyond x87/SSE -- the high 128-bit
 halves of the AVX/AVX2 YMM registers, the high 256-bit halves of the
 AVX512 ZMM registers or the high 16 ZMM registers, the obsolete BND
 registers, the future AMX/ACE tile data, &c. -- you will need to fish
 it out of uc->uc_mcontext.__fpregs.__xsaveptr/__xsavelen.  And
 avoiding the overlap meant the new logic in amd64/machdep.c -- and
 amd64/netbsd32_machdep.c and i386/machdep.c -- would be nice and
 simple and very clearly conditional on `if (process_needs_xsave_p())'.
 
 That meant, however, that there would be two locations where the
 FXSAVE area could live: one inside the ucontext_t, and one in the
 separate XSAVE area.
 
 `Not to worry,' I thought to myself, `because we can just keep copying
 out the ucontext_t FXSAVE area so existing debuggers can still read
 the state even if they don't know about the separate XSAVE area, and
 it'll all be hunky-dory, right?'
 
 Wrong!  FPU trap handlers (and debuggers) can substitute values in
 computations, so they may also need to write back registers.  And with
 two FXSAVE areas, which one does the kernel restore from?  Well, I
 picked the XSAVE one, not the ucontext_t one, again to keep it simple.
 
 So, perhaps we should combine the approaches:
 
 1. use the __xsaveptr/__xsavelen so that the MD cpu_setmcontext knows
    where to find the XSAVE area,
 AND
 2. make the XSAVE area overlap the ucontext_t so the FXSAVE sections
    of the two coincide, with some careful attention to stack pointer
    arithmetic and alignment in sendsig_siginfo.
 
 This way:
 
 - Programs that hordes of engineers around the world have rushed to
   adapt to NetBSD 11.0 by using __xsaveptr/__xsavelen will still work
   in NetBSD 11.1 and NetBSD 12, i.e., we don't break compatibility
   _again_ in quick succession.
 
 - Programs whose inexcusibly lazy maintainers are way behind the times
   and STILL TODAY haven't done a thing to adapt to NetBSD 11.0 will
   also continue to work even if they newly use AVX when they didn't
   before, i.e., we restore `compatibility' for programs that never
   worked right in the first place on NetBSD.
 
   And for NetBSD>11.0, they will be able to just cast __fpregs to an
   XSAVE area for access to >=3DAVX state, without having to go through
   the __xsaveptr/__xsavelen business altogether.
 
 - Programs that don't use extended CPU state beyond x87/SSE remain
   unaffected.
 
 - We can pull up the changes -- with the array-to-union change under
   #ifdef _KERNEL -- to netbsd-10 and netbsd-9 to make AVX register
   access work reliably in the face of signal handlers without breaking
   the API or affecting `compatibility' of programs using AVX registers
   that never worked right in the first place on NetBSD.