* [PATCH 0/2] xtensa: fix pt_regs to cover user exception frame
@ 2022-03-06 6:44 Max Filippov
2022-03-06 6:44 ` [PATCH 1/2] xtensa: rename PT_SIZE to PT_KERNEL_SIZE Max Filippov
2022-03-06 6:44 ` [PATCH 2/2] xtensa: use XCHAL_NUM_AREGS as pt_regs::areg size Max Filippov
0 siblings, 2 replies; 5+ messages in thread
From: Max Filippov @ 2022-03-06 6:44 UTC (permalink / raw)
To: linux-xtensa
Cc: Chris Zankel, linux-kernel, Kees Cook, linux-hardening, Max Filippov
Hello,
this series updates xtensa struct pt_regs so that it covers the whole
user exception frame fixing kernel build issue visible in the current
linux-next.
Max Filippov (2):
xtensa: rename PT_SIZE to PT_KERNEL_SIZE
xtensa: use XCHAL_NUM_AREGS as pt_regs::areg size
arch/xtensa/include/asm/ptrace.h | 7 +++----
arch/xtensa/kernel/asm-offsets.c | 2 +-
arch/xtensa/kernel/entry.S | 14 +++++++-------
arch/xtensa/kernel/process.c | 10 ----------
arch/xtensa/kernel/vectors.S | 4 ++--
5 files changed, 13 insertions(+), 24 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] xtensa: rename PT_SIZE to PT_KERNEL_SIZE
2022-03-06 6:44 [PATCH 0/2] xtensa: fix pt_regs to cover user exception frame Max Filippov
@ 2022-03-06 6:44 ` Max Filippov
2022-03-07 19:57 ` Kees Cook
2022-03-06 6:44 ` [PATCH 2/2] xtensa: use XCHAL_NUM_AREGS as pt_regs::areg size Max Filippov
1 sibling, 1 reply; 5+ messages in thread
From: Max Filippov @ 2022-03-06 6:44 UTC (permalink / raw)
To: linux-xtensa
Cc: Chris Zankel, linux-kernel, Kees Cook, linux-hardening, Max Filippov
PT_SIZE is used by the xtensa port to designate kernel exception frame
size. In preparation for struct pt_regs size change rename PT_SIZE to
PT_KERNEL_SIZE for clarity and change its definition to always cover
only the kernel exception frame.
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
arch/xtensa/kernel/asm-offsets.c | 2 +-
arch/xtensa/kernel/entry.S | 14 +++++++-------
arch/xtensa/kernel/vectors.S | 4 ++--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/xtensa/kernel/asm-offsets.c b/arch/xtensa/kernel/asm-offsets.c
index f1fd1390d069..37278e2785fb 100644
--- a/arch/xtensa/kernel/asm-offsets.c
+++ b/arch/xtensa/kernel/asm-offsets.c
@@ -63,7 +63,7 @@ int main(void)
DEFINE(PT_AREG15, offsetof (struct pt_regs, areg[15]));
DEFINE(PT_WINDOWBASE, offsetof (struct pt_regs, windowbase));
DEFINE(PT_WINDOWSTART, offsetof(struct pt_regs, windowstart));
- DEFINE(PT_SIZE, sizeof(struct pt_regs));
+ DEFINE(PT_KERNEL_SIZE, offsetof(struct pt_regs, areg[16]));
DEFINE(PT_AREG_END, offsetof (struct pt_regs, areg[XCHAL_NUM_AREGS]));
DEFINE(PT_USER_SIZE, offsetof(struct pt_regs, areg[XCHAL_NUM_AREGS]));
DEFINE(PT_XTREGS_OPT, offsetof(struct pt_regs, xtregs_opt));
diff --git a/arch/xtensa/kernel/entry.S b/arch/xtensa/kernel/entry.S
index a1029a5b6a1d..77a7c8da3ff5 100644
--- a/arch/xtensa/kernel/entry.S
+++ b/arch/xtensa/kernel/entry.S
@@ -341,8 +341,8 @@ KABI_W _bbsi.l a2, 3, 1f
/* Copy spill slots of a0 and a1 to imitate movsp
* in order to keep exception stack continuous
*/
- l32i a3, a1, PT_SIZE
- l32i a0, a1, PT_SIZE + 4
+ l32i a3, a1, PT_KERNEL_SIZE
+ l32i a0, a1, PT_KERNEL_SIZE + 4
s32e a3, a1, -16
s32e a0, a1, -12
#endif
@@ -706,12 +706,12 @@ kernel_exception_exit:
addi a0, a1, -16
l32i a3, a0, 0
l32i a4, a0, 4
- s32i a3, a1, PT_SIZE+0
- s32i a4, a1, PT_SIZE+4
+ s32i a3, a1, PT_KERNEL_SIZE + 0
+ s32i a4, a1, PT_KERNEL_SIZE + 4
l32i a3, a0, 8
l32i a4, a0, 12
- s32i a3, a1, PT_SIZE+8
- s32i a4, a1, PT_SIZE+12
+ s32i a3, a1, PT_KERNEL_SIZE + 8
+ s32i a4, a1, PT_KERNEL_SIZE + 12
/* Common exception exit.
* We restore the special register and the current window frame, and
@@ -821,7 +821,7 @@ ENTRY(debug_exception)
bbsi.l a2, PS_UM_BIT, 2f # jump if user mode
- addi a2, a1, -16-PT_SIZE # assume kernel stack
+ addi a2, a1, -16 - PT_KERNEL_SIZE # assume kernel stack
3:
l32i a0, a3, DT_DEBUG_SAVE
s32i a1, a2, PT_AREG1
diff --git a/arch/xtensa/kernel/vectors.S b/arch/xtensa/kernel/vectors.S
index 407ece204e7c..1073fe4a584d 100644
--- a/arch/xtensa/kernel/vectors.S
+++ b/arch/xtensa/kernel/vectors.S
@@ -88,7 +88,7 @@ ENDPROC(_UserExceptionVector)
* Kernel exception vector. (Exceptions with PS.UM == 0, PS.EXCM == 0)
*
* We get this exception when we were already in kernel space.
- * We decrement the current stack pointer (kernel) by PT_SIZE and
+ * We decrement the current stack pointer (kernel) by PT_KERNEL_SIZE and
* jump to the first-level handler associated with the exception cause.
*
* Note: we need to preserve space for the spill region.
@@ -100,7 +100,7 @@ ENTRY(_KernelExceptionVector)
xsr a3, excsave1 # save a3, and get dispatch table
wsr a2, depc # save a2
- addi a2, a1, -16-PT_SIZE # adjust stack pointer
+ addi a2, a1, -16 - PT_KERNEL_SIZE # adjust stack pointer
s32i a0, a2, PT_AREG0 # save a0 to ESF
rsr a0, exccause # retrieve exception cause
s32i a0, a2, PT_DEPC # mark it as a regular exception
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] xtensa: use XCHAL_NUM_AREGS as pt_regs::areg size
2022-03-06 6:44 [PATCH 0/2] xtensa: fix pt_regs to cover user exception frame Max Filippov
2022-03-06 6:44 ` [PATCH 1/2] xtensa: rename PT_SIZE to PT_KERNEL_SIZE Max Filippov
@ 2022-03-06 6:44 ` Max Filippov
2022-03-07 19:56 ` Kees Cook
1 sibling, 1 reply; 5+ messages in thread
From: Max Filippov @ 2022-03-06 6:44 UTC (permalink / raw)
To: linux-xtensa
Cc: Chris Zankel, linux-kernel, Kees Cook, linux-hardening, Max Filippov
struct pt_regs is used to access both kernel and user exception frames.
User exception frames may contain up to XCHAL_NUM_AREG registers that
task creation and signal delivery code may access, but pt_regs::areg
array has only 16 entries that cover only the kernel exception frame.
This results in the following build error:
arch/xtensa/kernel/process.c: In function 'copy_thread':
arch/xtensa/kernel/process.c:262:52: error: array subscript 53 is above
array bounds of 'long unsigned int[16]' [-Werror=array-bounds]
262 | put_user(regs->areg[caller_ars+1],
Change struct pt_regs::areg size to XCHAL_NUM_AREGS so that it covers
the whole user exception frame. Adjust task_pt_regs and drop additional
register copying code from copy_thread now that the whole user exception
stack frame is copied.
Reported-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
arch/xtensa/include/asm/ptrace.h | 7 +++----
arch/xtensa/kernel/process.c | 10 ----------
2 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/arch/xtensa/include/asm/ptrace.h b/arch/xtensa/include/asm/ptrace.h
index b109416dc07e..308f209a4740 100644
--- a/arch/xtensa/include/asm/ptrace.h
+++ b/arch/xtensa/include/asm/ptrace.h
@@ -44,6 +44,7 @@
#ifndef __ASSEMBLY__
#include <asm/coprocessor.h>
+#include <asm/core.h>
/*
* This struct defines the way the registers are stored on the
@@ -77,14 +78,12 @@ struct pt_regs {
/* current register frame.
* Note: The ESF for kernel exceptions ends after 16 registers!
*/
- unsigned long areg[16];
+ unsigned long areg[XCHAL_NUM_AREGS];
};
-#include <asm/core.h>
-
# define arch_has_single_step() (1)
# define task_pt_regs(tsk) ((struct pt_regs*) \
- (task_stack_page(tsk) + KERNEL_STACK_SIZE - (XCHAL_NUM_AREGS-16)*4) - 1)
+ (task_stack_page(tsk) + KERNEL_STACK_SIZE) - 1)
# define user_mode(regs) (((regs)->ps & 0x00000020)!=0)
# define instruction_pointer(regs) ((regs)->pc)
# define return_pointer(regs) (MAKE_PC_FROM_RA((regs)->areg[0], \
diff --git a/arch/xtensa/kernel/process.c b/arch/xtensa/kernel/process.c
index bd80df890b1e..e8bfbca5f001 100644
--- a/arch/xtensa/kernel/process.c
+++ b/arch/xtensa/kernel/process.c
@@ -232,10 +232,6 @@ int copy_thread(unsigned long clone_flags, unsigned long usp_thread_fn,
p->thread.ra = MAKE_RA_FOR_CALL(
(unsigned long)ret_from_fork, 0x1);
- /* This does not copy all the regs.
- * In a bout of brilliance or madness,
- * ARs beyond a0-a15 exist past the end of the struct.
- */
*childregs = *regs;
childregs->areg[1] = usp;
childregs->areg[2] = 0;
@@ -265,14 +261,8 @@ int copy_thread(unsigned long clone_flags, unsigned long usp_thread_fn,
childregs->wmask = 1;
childregs->windowstart = 1;
childregs->windowbase = 0;
- } else {
- int len = childregs->wmask & ~0xf;
- memcpy(&childregs->areg[XCHAL_NUM_AREGS - len/4],
- ®s->areg[XCHAL_NUM_AREGS - len/4], len);
}
- childregs->syscall = regs->syscall;
-
if (clone_flags & CLONE_SETTLS)
childregs->threadptr = tls;
} else {
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] xtensa: use XCHAL_NUM_AREGS as pt_regs::areg size
2022-03-06 6:44 ` [PATCH 2/2] xtensa: use XCHAL_NUM_AREGS as pt_regs::areg size Max Filippov
@ 2022-03-07 19:56 ` Kees Cook
0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2022-03-07 19:56 UTC (permalink / raw)
To: Max Filippov; +Cc: linux-xtensa, Chris Zankel, linux-kernel, linux-hardening
On Sat, Mar 05, 2022 at 10:44:35PM -0800, Max Filippov wrote:
> struct pt_regs is used to access both kernel and user exception frames.
> User exception frames may contain up to XCHAL_NUM_AREG registers that
> task creation and signal delivery code may access, but pt_regs::areg
> array has only 16 entries that cover only the kernel exception frame.
> This results in the following build error:
>
> arch/xtensa/kernel/process.c: In function 'copy_thread':
> arch/xtensa/kernel/process.c:262:52: error: array subscript 53 is above
> array bounds of 'long unsigned int[16]' [-Werror=array-bounds]
> 262 | put_user(regs->areg[caller_ars+1],
>
> Change struct pt_regs::areg size to XCHAL_NUM_AREGS so that it covers
> the whole user exception frame. Adjust task_pt_regs and drop additional
> register copying code from copy_thread now that the whole user exception
> stack frame is copied.
>
> Reported-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
> arch/xtensa/include/asm/ptrace.h | 7 +++----
> arch/xtensa/kernel/process.c | 10 ----------
> 2 files changed, 3 insertions(+), 14 deletions(-)
^^^^^^^^^^^^
Well that's always nice to see in a fix. :) Thanks for digging into
this!
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] xtensa: rename PT_SIZE to PT_KERNEL_SIZE
2022-03-06 6:44 ` [PATCH 1/2] xtensa: rename PT_SIZE to PT_KERNEL_SIZE Max Filippov
@ 2022-03-07 19:57 ` Kees Cook
0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2022-03-07 19:57 UTC (permalink / raw)
To: Max Filippov; +Cc: linux-xtensa, Chris Zankel, linux-kernel, linux-hardening
On Sat, Mar 05, 2022 at 10:44:34PM -0800, Max Filippov wrote:
> PT_SIZE is used by the xtensa port to designate kernel exception frame
> size. In preparation for struct pt_regs size change rename PT_SIZE to
> PT_KERNEL_SIZE for clarity and change its definition to always cover
> only the kernel exception frame.
>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-07 19:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-06 6:44 [PATCH 0/2] xtensa: fix pt_regs to cover user exception frame Max Filippov
2022-03-06 6:44 ` [PATCH 1/2] xtensa: rename PT_SIZE to PT_KERNEL_SIZE Max Filippov
2022-03-07 19:57 ` Kees Cook
2022-03-06 6:44 ` [PATCH 2/2] xtensa: use XCHAL_NUM_AREGS as pt_regs::areg size Max Filippov
2022-03-07 19:56 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).