Excerpts from Sachin Sant's message of November 25, 2021 9:37 pm:
> While running sigfuz powerpc self test following warning is seen on a Power
> 10 LPAR.
> This warning is seen only with CONFIG_PPC_IRQ_SOFT_MASK_DEBUG=y
> The kernel eventually panics.
>
> [ 1032.912973] sigfuz[2061671]: bad frame in rt_sigreturn: 0000000097830b2d
> nip 00007fff9d9a0470 lr 00007fff9d9a0464
> [ 1032.913430] ------------[ cut here ]------------
> [ 1032.913455] WARNING: CPU: 6 PID: 2061674 at
> arch/powerpc/kernel/interrupt_64.S:34 system_call_common+0x150/0x268
> [ 1032.913482] Modules linked in: bonding tls nft_fib_inet nft_fib_ipv4
> nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject
> nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set
> nf_tables rfkill libcrc32c nfnetlink sunrpc xts vmx_crypto pseries_rng
> ip_tables ext4 mbcache jbd2 dm_service_time sd_mod t10_pi sg ibmvfc
> scsi_transport_fc ibmveth dm_multipath dm_mirror dm_region_hash dm_log dm_mod
> fuse
> [ 1032.913587] CPU: 6 PID: 2061674 Comm: sigfuz Not tainted
> 5.16.0-rc2-g95c6ab13ec7e #1
> [ 1032.913612] NIP: c00000000000c730 LR: 0000000045faa436 CTR:
> 0000000000000000
> [ 1032.913636] REGS: c00000000c7e7b70 TRAP: 0700 Not tainted
> (5.16.0-rc2-g95c6ab13ec7e)
> [ 1032.913652] MSR: 800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR:
> 28004474 XER: 20040000
> [ 1032.913679] CFAR: c00000000003a2d4 IRQMASK: 0
> [ 1032.913679] GPR00: c00000000000c6d8 c00000000c7e7e10 000000002fcdac67
> 0000000000000800
> [ 1032.913679] GPR04: 0000000060d67006 800000000280f032 0000000045faa436
> c0000001eb3d4c00
> [ 1032.913679] GPR08: 800000000280f032 0000000000000001 0000000000000001
> 0000000060d67004
> [ 1032.913679] GPR12: 0000000060d67006 c00000077fdf2300 0000000000000000
> 00007fff9da00000
SRR0 == r11, regs->nip == r12
I wonder if this is just that SRR0 does not implement the bottom 2 bits
so the check fails when the signal context sets them. Hopefully the
panic is just due to this warning 0x700 program check hitting at a bad
time.
We could always adjust the debug check but maybe something like this
would keep those bits clear which might be cleaner.
Thanks,
Nick
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 3e053e2fd6b6..92a3a6982813 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -116,7 +116,7 @@ __unsafe_restore_general_regs(struct pt_regs *regs, struct
mcontext __user *sr)
int i;
for (i = 0; i <= PT_RESULT; i++) {
- if ((i == PT_MSR) || (i == PT_SOFTE))
+ if ((i == PT_NIP) || (i == PT_MSR) || (i == PT_SOFTE))
continue;
unsafe_get_user(gregs[i], &sr->mc_gregs[i], failed);
}
@@ -156,7 +156,7 @@ static __always_inline
int __unsafe_restore_general_regs(struct pt_regs *regs, struct mcontext __user
*sr)
{
/* copy up to but not including MSR */
- unsafe_copy_from_user(regs, &sr->mc_gregs, PT_MSR * sizeof(elf_greg_t),
failed);
+ unsafe_copy_from_user(regs, &sr->mc_gregs, PT_NIP * sizeof(elf_greg_t),
failed);
/* copy from orig_r3 (the word after the MSR) up to the end */
unsafe_copy_from_user(®s->orig_gpr3, &sr->mc_gregs[PT_ORIG_R3],
@@ -458,7 +458,7 @@ static long restore_user_regs(struct pt_regs *regs,
struct mcontext __user *sr, int sig)
{
unsigned int save_r2 = 0;
- unsigned long msr;
+ unsigned long nip, msr;
#ifdef CONFIG_VSX
int i;
#endif
@@ -473,6 +473,8 @@ static long restore_user_regs(struct pt_regs *regs,
save_r2 = (unsigned int)regs->gpr[2];
unsafe_restore_general_regs(regs, sr, failed);
set_trap_norestart(regs);
+ unsafe_get_user(nip, &sr->mc_gregs[PT_NIP], failed);
+ regs_set_return_ip(regs, nip & ~3);
unsafe_get_user(msr, &sr->mc_gregs[PT_MSR], failed);
if (!sig)
regs->gpr[2] = (unsigned long) save_r2;
@@ -560,7 +562,7 @@ static long restore_tm_user_regs(struct pt_regs *regs,
struct mcontext __user *sr,
struct mcontext __user *tm_sr)
{
- unsigned long msr, msr_hi;
+ unsigned long nip, msr, msr_hi;
int i;
if (tm_suspend_disabled)
@@ -576,7 +578,8 @@ static long restore_tm_user_regs(struct pt_regs *regs,
return 1;
unsafe_restore_general_regs(¤t->thread.ckpt_regs, sr, failed);
- unsafe_get_user(current->thread.tm_tfhar, &sr->mc_gregs[PT_NIP],
failed);
+ unsafe_get_user(nip, &sr->mc_gregs[PT_NIP], failed);
+ current->thread.tm_tfhar = nip & ~3;
unsafe_get_user(msr, &sr->mc_gregs[PT_MSR], failed);
/* Restore the previous little-endian mode */
@@ -646,6 +649,9 @@ static long restore_tm_user_regs(struct pt_regs *regs,
current->thread.used_vsr = true;
}
+ unsafe_get_user(nip, &tm_sr->mc_gregs[PT_NIP], failed);
+ regs_set_return_ip(regs, nip & ~3);
+
/* Get the top half of the MSR from the user context */
unsafe_get_user(msr_hi, &tm_sr->mc_gregs[PT_MSR], failed);
msr_hi <<= 32;
@@ -801,7 +807,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t
*oldset,
regs->gpr[4] = (unsigned long)&frame->info;
regs->gpr[5] = (unsigned long)&frame->uc;
regs->gpr[6] = (unsigned long)frame;
- regs_set_return_ip(regs, (unsigned long) ksig->ka.sa.sa_handler);
+ regs_set_return_ip(regs, (unsigned long) ksig->ka.sa.sa_handler & ~3);
/* enter the signal handler in native-endian mode */
regs_set_return_msr(regs, (regs->msr & ~MSR_LE) | (MSR_KERNEL &
MSR_LE));
@@ -889,7 +895,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
regs->gpr[1] = newsp;
regs->gpr[3] = ksig->sig;
regs->gpr[4] = (unsigned long) sc;
- regs_set_return_ip(regs, (unsigned long) ksig->ka.sa.sa_handler);
+ regs_set_return_ip(regs, (unsigned long) ksig->ka.sa.sa_handler & ~3);
/* enter the signal handler in native-endian mode */
regs_set_return_msr(regs, (regs->msr & ~MSR_LE) | (MSR_KERNEL &
MSR_LE));
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index d1e1fc0acbea..0327e5c79c36 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -336,7 +336,7 @@ static long notrace __unsafe_restore_sigcontext(struct
task_struct *tsk, sigset_
elf_vrreg_t __user *v_regs;
#endif
unsigned long save_r13 = 0;
- unsigned long msr;
+ unsigned long nip, msr;
struct pt_regs *regs = tsk->thread.regs;
#ifdef CONFIG_VSX
int i;
@@ -350,7 +350,8 @@ static long notrace __unsafe_restore_sigcontext(struct
task_struct *tsk, sigset_
/* copy the GPRs */
unsafe_copy_from_user(regs->gpr, sc->gp_regs, sizeof(regs->gpr),
efault_out);
- unsafe_get_user(regs->nip, &sc->gp_regs[PT_NIP], efault_out);
+ unsafe_get_user(nip, &sc->gp_regs[PT_NIP], efault_out);
+ regs_set_return_ip(regs, nip & ~3);
/* get MSR separately, transfer the LE bit if doing signal return */
unsafe_get_user(msr, &sc->gp_regs[PT_MSR], efault_out);
if (sig)
@@ -434,7 +435,7 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,
elf_vrreg_t __user *v_regs, *tm_v_regs;
#endif
unsigned long err = 0;
- unsigned long msr;
+ unsigned long nip, msr;
struct pt_regs *regs = tsk->thread.regs;
#ifdef CONFIG_VSX
int i;
@@ -458,8 +459,10 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,
* For the case of getting a signal and simply returning from it,
* we don't need to re-copy them here.
*/
- err |= __get_user(regs->nip, &tm_sc->gp_regs[PT_NIP]);
- err |= __get_user(tsk->thread.tm_tfhar, &sc->gp_regs[PT_NIP]);
+ err |= __get_user(nip, &tm_sc->gp_regs[PT_NIP]);
+ regs_set_return_ip(regs, nip & ~3);
+ err |= __get_user(nip, &sc->gp_regs[PT_NIP]);
+ tsk->thread.tm_tfhar = nip & ~3;
/* get MSR separately, transfer the LE bit if doing signal return */
err |= __get_user(msr, &sc->gp_regs[PT_MSR]);
