Hi Finn,
On 2/05/23 18:50, Finn Thain wrote:
The 68030 Users Manual says that address errors occur immediately they
are detected during instruction prefetch. The instruction pipeline allows
prefetch to overlap with other instructions which means an address error
can be taken during execution of a different instruction.
Both a bus error and an address error may produce a format 0xB exception
frame. Regarding those frames, the UM says the PC contained therein has
the "address of instruction in execution when fault occurred -- may not be
the instruction which generated the faulted bus cycle".
The UM also states that:
"An address error exception occurs when the processor attempts to
prefetch an instruction
from an odd address. This exception is similar to a bus error exception,
but is internally
initiated. A bus cycle is not executed, and the processor begins
exception processing
immediately." (8.1.3)
In the bus error case, the USP in that frame is not reliable (like the
PC). We should not rely on it in the address error case. The address error
The instruction causing the fault can't have modified USP yet. So the
case that you are concerned about here is where the instruction in
execution is e.g. a moveml using USP as one of the data addresses
(making USP unreliable) and the instruction faulting is one of the next
ones being decoded. Address properly aligned for the first instruction,
then no longer aligned for the next ones?
(You mentioned cpBcc or cpDBcc instructions in another mail - where did
you find that? It's not mentioned in my copy of the 030 UM...)
I'm not trying to say this all isn't necessary for address errors. I'm
just trying to understand why it is.
case always produces a SIGBUS. That signal may be caught and the
exception fixed up. If a debugger or emulator were to do so, this patch
would theoretically prevent user stack corruption.
Cc: Michael Schmitz <schmitzmic@xxxxxxxxx>
Cc: Andreas Schwab <schwab@xxxxxxxxxxxxxx>
Link: https://lore.kernel.org/linux-m68k/20230429080410.8993-1-schmitzmic@xxxxxxxxx/
Co-developed-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxx>
---
Michael, as co-developer, this will need your signed-off-by.
I believe this patch is currently our best option to prevent user stack
corruption by signal frames in the bus error case (after Andreas pointed
out a problem with your patch 1).
I've also been unable to reproduce the out-of-memory condition with a
stack gap size of 256 in my recent tests, so I can no longer see a
drawback compared to my (immensely more complex) similar patch.
So even though I'm still a bit thick on the address error case:
Signed-off-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
---
arch/m68k/kernel/signal.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
index b9f6908a31bc..8aeafbb083f7 100644
--- a/arch/m68k/kernel/signal.c
+++ b/arch/m68k/kernel/signal.c
@@ -858,11 +858,16 @@ static inline int rt_setup_ucontext(struct ucontext __user *uc, struct pt_regs *
}
static inline void __user *
-get_sigframe(struct ksignal *ksig, size_t frame_size)
+get_sigframe(struct ksignal *ksig, struct pt_regs *tregs, size_t frame_size)
{
unsigned long usp = sigsp(rdusp(), ksig);
+ unsigned long gap = 0;
- return (void __user *)((usp - frame_size) & -8UL);
+ if (CPU_IS_020_OR_030 && tregs->format == 0xb)
+ /* USP is unreliable so use worst-case value */
+ gap = 256;
+
+ return (void __user *)((usp - gap - frame_size) & -8UL);
}
static int setup_frame(struct ksignal *ksig, sigset_t *set,
@@ -880,7 +885,7 @@ static int setup_frame(struct ksignal *ksig, sigset_t *set,
return -EFAULT;
}
- frame = get_sigframe(ksig, sizeof(*frame) + fsize);
+ frame = get_sigframe(ksig, tregs, sizeof(*frame) + fsize);
if (fsize)
err |= copy_to_user (frame + 1, regs + 1, fsize);
@@ -952,7 +957,7 @@ static int setup_rt_frame(struct ksignal *ksig, sigset_t *set,
return -EFAULT;
}
- frame = get_sigframe(ksig, sizeof(*frame));
+ frame = get_sigframe(ksig, tregs, sizeof(*frame));
if (fsize)
err |= copy_to_user (&frame->uc.uc_extra, regs + 1, fsize);
