Re: signal delivery, was Re: reliable reproducer

Hi Andreas, Finn and Michael,

With no patches installed, I see stack smashing on my IIci after one
boot using a customized 6.2.12 kernel, no initrd, and Debian's SID and
dash. Also, Finn's test program (compiled using gcc 12) generates
illegal instructions, as expected.

With the patch below installed, I didn't see any stack smashing after
two boots on the same IIci using the same configuration, and Finn's test
program did not generate illegal instructions.

I have the serial console logs if you need them. If that was a valid
test, I can confirm (if needed) that I get the same results on a IIfx,
LC III or SE/30 (or a PowerBook 170 if I want to maximize my pain).

Thanks to all for your work debugging this issue!

-Stan

-----

On 4/25/23 5:53 AM, Andreas Schwab wrote:
> Please try this patch.  Signal delivery should only happen at insn
> boundaries, but due to the way the 030 handles return from bus error
> exceptions (the insn is resumed, not restarted like on the 040/060) the
> kernel may do it in the middle of the faulting insn.
> 
> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
> index 4dd2fd7acba9..6c09a5710728 100644
> --- a/arch/m68k/kernel/entry.S
> +++ b/arch/m68k/kernel/entry.S
> @@ -117,7 +117,11 @@ ENTRY(buserr)
>  	movel	%sp,%sp@-		| stack frame pointer argument
>  	jbsr	buserr_c
>  	addql	#4,%sp
> -	jra	ret_from_exception
> +	| don't do signal delivery when interrupted while insn is in progress
> +	| (on the 020/030)
> +	tstl	%d0
> +	jeq	ret_from_exception
> +	RESTORE_ALL
>  
>  ENTRY(trap)
>  	SAVE_ALL_INT
> diff --git a/arch/m68k/kernel/traps.c b/arch/m68k/kernel/traps.c
> index a700807c9b6d..40fc408d1333 100644
> --- a/arch/m68k/kernel/traps.c
> +++ b/arch/m68k/kernel/traps.c
> @@ -751,8 +751,10 @@ static inline void access_errorcf(unsigned int fs, struct frame *fp)
>  }
>  #endif /* CONFIG_COLDFIRE CONFIG_MMU */
>  
> -asmlinkage void buserr_c(struct frame *fp)
> +asmlinkage int buserr_c(struct frame *fp)
>  {
> +	int not_insn_boundary = 0;
> +
>  	/* Only set esp0 if coming from user mode */
>  	if (user_mode(&fp->ptregs))
>  		current->thread.esp0 = (unsigned long) fp;
> @@ -793,8 +795,9 @@ asmlinkage void buserr_c(struct frame *fp)
>  	  break;
>  #endif
>  #if defined (CPU_M68020_OR_M68030)
> -	case 0xa:
>  	case 0xb:
> +	  not_insn_boundary = 1;
> +	case 0xa:
>  	  bus_error030 (fp);
>  	  break;
>  #endif
> @@ -803,6 +806,8 @@ asmlinkage void buserr_c(struct frame *fp)
>  	  pr_debug("Unknown SIGSEGV - 4\n");
>  	  force_sig(SIGSEGV);
>  	}
> +
> +	return not_insn_boundary;
>  }
>  
>  
>