Skip to content

Commit 535ca24

Browse files
riastradhriastradh
riastradh
authored and
riastradh
committed
mips: Align stack pointer on entry to signal handler.
Based on a patch by rin@. The variant approach I took puts the stack frame allocation and alignment logic in one place (getframe, used by sendsig_siginfo for native (n64, on mips), netbsd32_sendsig_siginfo for compat32 (n32/o32, on mips), and sendsig_sigcontext (compat 1.6)) and reduces the chance of provoking compiler exploitation of undefined behaviour by doing arithmetic in uintptr_t rather than in pointers to large aligned structs. This also ensures the resulting pointer is aligned for the object (struct siginfo_sigframe, struct siginfo_sigframe32, struct sigcontext), not just for the ABI stack alignment. PR kern/59327: user stack pointer is not aligned properly
1 parent 23f073d commit 535ca24

6 files changed

Lines changed: 37 additions & 30 deletions

File tree

‎sys/arch/mips/include/frame.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: frame.h,v 1.12 2021/03/29 01:46:26 simonb Exp $ */
1+
/* $NetBSD: frame.h,v 1.13 2025/04/25 00:26:58 riastradh Exp $ */
22

33
/*
44
* Copyright (c) 2001 The NetBSD Foundation, Inc.
@@ -40,8 +40,9 @@
4040
#endif
4141

4242
#include <sys/signal.h>
43+
#include <sys/stddef.h>
4344

44-
void *getframe(struct lwp *, int, int *);
45+
void *getframe(struct lwp *, int, int *, size_t, size_t);
4546
#define lwp_trapframe(l) ((l)->l_md.md_utf)
4647

4748
#if defined(COMPAT_16) || defined(COMPAT_ULTRIX)

‎sys/arch/mips/include/mips_param.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: mips_param.h,v 1.53 2025/04/20 22:32:25 riastradh Exp $ */
1+
/* $NetBSD: mips_param.h,v 1.54 2025/04/25 00:26:58 riastradh Exp $ */
22

33
/*-
44
* Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -82,6 +82,8 @@
8282
#define STACK_ALIGNBYTES (8 - 1)
8383
#endif
8484

85+
#define STACK_ALIGNBYTES_O32 (8 - 1)
86+
8587
#define ALIGNBYTES32 (sizeof(double) - 1)
8688
#define ALIGN32(p) (((uintptr_t)(p) + ALIGNBYTES32) &~ALIGNBYTES32)
8789

‎sys/arch/mips/mips/compat_16_machdep.c‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: compat_16_machdep.c,v 1.23 2021/10/27 04:15:00 thorpej Exp $ */
1+
/* $NetBSD: compat_16_machdep.c,v 1.24 2025/04/25 00:26:59 riastradh Exp $ */
22

33
/*-
44
* Copyright (c) 1998, 2001 The NetBSD Foundation, Inc.
@@ -45,7 +45,7 @@
4545

4646
#include <sys/cdefs.h> /* RCS ID & Copyright macro defns */
4747

48-
__KERNEL_RCSID(0, "$NetBSD: compat_16_machdep.c,v 1.23 2021/10/27 04:15:00 thorpej Exp $");
48+
__KERNEL_RCSID(0, "$NetBSD: compat_16_machdep.c,v 1.24 2025/04/25 00:26:59 riastradh Exp $");
4949

5050
#ifdef _KERNEL_OPT
5151
#include "opt_cputype.h"
@@ -96,7 +96,8 @@ sendsig_sigcontext(const ksiginfo_t *ksi, const sigset_t *returnmask)
9696
struct sigacts * const ps = p->p_sigacts;
9797
struct pcb * const pcb = lwp_getpcb(l);
9898
int onstack, error;
99-
struct sigcontext *scp = getframe(l, sig, &onstack);
99+
struct sigcontext *scp = getframe(l, sig, &onstack,
100+
sizeof(*scp), _Alignof(*scp));
100101
struct sigcontext ksc;
101102
struct trapframe * const tf = l->l_md.md_utf;
102103
sig_t catcher = SIGACTION(p, sig).sa_handler;
@@ -106,8 +107,6 @@ sendsig_sigcontext(const ksiginfo_t *ksi, const sigset_t *returnmask)
106107
sigexit(l, SIGILL);
107108
#endif
108109

109-
scp--;
110-
111110
#ifdef DEBUG
112111
if ((sigdebug & SDB_FOLLOW) ||
113112
((sigdebug & SDB_KSTACK) && p->p_pid == sigpid))

‎sys/arch/mips/mips/netbsd32_machdep.c‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: netbsd32_machdep.c,v 1.23 2021/11/06 20:42:56 thorpej Exp $ */
1+
/* $NetBSD: netbsd32_machdep.c,v 1.24 2025/04/25 00:26:59 riastradh Exp $ */
22

33
/*-
44
* Copyright (c) 2009 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
3030
*/
3131

3232
#include <sys/cdefs.h>
33-
__KERNEL_RCSID(0, "$NetBSD: netbsd32_machdep.c,v 1.23 2021/11/06 20:42:56 thorpej Exp $");
33+
__KERNEL_RCSID(0, "$NetBSD: netbsd32_machdep.c,v 1.24 2025/04/25 00:26:59 riastradh Exp $");
3434

3535
#include "opt_compat_netbsd.h"
3636

@@ -94,14 +94,13 @@ netbsd32_sendsig_siginfo(const ksiginfo_t *ksi, const sigset_t *mask)
9494
struct sigacts * const ps = p->p_sigacts;
9595
int onstack, error;
9696
int sig = ksi->ksi_signo;
97-
struct sigframe_siginfo32 *sfp = getframe(l, sig, &onstack);
97+
struct sigframe_siginfo32 *sfp = getframe(l, sig, &onstack,
98+
sizeof(*sfp), _Alignof(*sfp));
9899
struct sigframe_siginfo32 sf;
99100
struct trapframe * const tf = l->l_md.md_utf;
100101
size_t sfsz;
101102
sig_t catcher = SIGACTION(p, sig).sa_handler;
102103

103-
sfp--;
104-
105104
memset(&sf, 0, sizeof(sf));
106105
netbsd32_si_to_si32(&sf.sf_si, (const siginfo_t *)&ksi->ksi_info);
107106

‎sys/arch/mips/mips/sig_machdep.c‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: sig_machdep.c,v 1.25 2024/04/14 07:56:45 skrll Exp $ */
1+
/* $NetBSD: sig_machdep.c,v 1.26 2025/04/25 00:26:59 riastradh Exp $ */
22

33
/*-
44
* Copyright (c) 2003 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
3131

3232
#include <sys/cdefs.h> /* RCS ID & Copyright macro defns */
3333

34-
__KERNEL_RCSID(0, "$NetBSD: sig_machdep.c,v 1.25 2024/04/14 07:56:45 skrll Exp $");
34+
__KERNEL_RCSID(0, "$NetBSD: sig_machdep.c,v 1.26 2025/04/25 00:26:59 riastradh Exp $");
3535

3636
#include "opt_cputype.h"
3737

@@ -49,17 +49,29 @@ __KERNEL_RCSID(0, "$NetBSD: sig_machdep.c,v 1.25 2024/04/14 07:56:45 skrll Exp $
4949
#include <mips/locore.h>
5050

5151
void *
52-
getframe(struct lwp *l, int sig, int *onstack)
52+
getframe(struct lwp *l, int sig, int *onstack, size_t size, size_t align)
5353
{
5454
struct proc * const p = l->l_proc;
5555
struct trapframe * const tf = l->l_md.md_utf;
56+
uintptr_t sp;
57+
58+
KASSERT((align & (align - 1)) == 0);
5659

5760
/* Do we need to jump onto the signal stack? */
5861
*onstack = (l->l_sigstk.ss_flags & (SS_DISABLE | SS_ONSTACK)) == 0
5962
&& (SIGACTION(p, sig).sa_flags & SA_ONSTACK) != 0;
6063
if (*onstack)
61-
return (char *)l->l_sigstk.ss_sp + l->l_sigstk.ss_size;
62-
return (void *)(intptr_t)tf->tf_regs[_R_SP];
64+
sp = (uintptr_t)l->l_sigstk.ss_sp + l->l_sigstk.ss_size;
65+
else
66+
sp = (uintptr_t)tf->tf_regs[_R_SP];
67+
sp -= size;
68+
#ifndef __mips_o32
69+
if (p->p_md.md_abi == _MIPS_BSD_API_O32)
70+
sp &= ~(STACK_ALIGNBYTES_O32 | (align - 1));
71+
else
72+
#endif
73+
sp &= ~(STACK_ALIGNBYTES | (align - 1));
74+
return (void *)sp;
6375
}
6476

6577
struct sigframe_siginfo {
@@ -79,12 +91,11 @@ sendsig_siginfo(const ksiginfo_t *ksi, const sigset_t *mask)
7991
struct trapframe * const tf = l->l_md.md_utf;
8092
int onstack, error;
8193
const int signo = ksi->ksi_signo;
82-
struct sigframe_siginfo *sf = getframe(l, signo, &onstack);
94+
struct sigframe_siginfo *sf = getframe(l, signo, &onstack,
95+
sizeof(*sf), _Alignof(*sf));
8396
struct sigframe_siginfo ksf;
8497
const sig_t catcher = SIGACTION(p, signo).sa_handler;
8598

86-
sf--;
87-
8899
memset(&ksf, 0, sizeof(ksf));
89100
ksf.sf_si._info = ksi->ksi_info;
90101
ksf.sf_uc.uc_flags = _UC_SIGMASK

‎tests/kernel/t_signal_and_sp.c‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: t_signal_and_sp.c,v 1.14 2025/04/25 00:03:16 riastradh Exp $ */
1+
/* $NetBSD: t_signal_and_sp.c,v 1.15 2025/04/25 00:26:59 riastradh Exp $ */
22

33
/*
44
* Copyright (c) 2024 The NetBSD Foundation, Inc.
@@ -27,7 +27,7 @@
2727
*/
2828

2929
#include <sys/cdefs.h>
30-
__RCSID("$NetBSD: t_signal_and_sp.c,v 1.14 2025/04/25 00:03:16 riastradh Exp $");
30+
__RCSID("$NetBSD: t_signal_and_sp.c,v 1.15 2025/04/25 00:26:59 riastradh Exp $");
3131

3232
#include <sys/wait.h>
3333

@@ -384,11 +384,6 @@ ATF_TC_BODY(signalsp, tc)
384384
#if defined STACK_ALIGNBYTES && defined HAVE_SIGNALSPHANDLER
385385
struct sigaction sa;
386386

387-
#if defined __mips_n32 || defined __mips_n64 /* 16-byte, vs 8-byte o32 */
388-
atf_tc_expect_fail("PR kern/59327:"
389-
" user stack pointer is not aligned properly");
390-
#endif
391-
392387
memset(&sa, 0, sizeof(sa));
393388
sa.sa_handler = &signalsphandler;
394389
RL(sigaction(SIGUSR1, &sa, NULL));
@@ -431,7 +426,7 @@ ATF_TC_BODY(signalsp_sigaltstack, tc)
431426
fprintf(stderr, "stack @ [%p, %p)\n",
432427
stack, stack + SIGSTKSZ + STACK_ALIGNBYTES);
433428

434-
#if defined __alpha__ || defined __mips__
429+
#if defined __alpha__
435430
atf_tc_expect_fail("PR kern/59327:"
436431
" user stack pointer is not aligned properly");
437432
#endif
@@ -583,7 +578,7 @@ ATF_TC_BODY(misaligned_sp_and_signal, tc)
583578
sa.sa_handler = &signalsphandler;
584579
RL(sigaction(SIGALRM, &sa, NULL));
585580

586-
#if defined __alpha__ || defined __mips__
581+
#if defined __alpha__
587582
atf_tc_expect_fail("PR kern/58149:"
588583
" Cannot return from a signal handler"
589584
" if SP was misaligned when the signal arrived");

0 commit comments

Comments
 (0)