Re: signal delivery, was Re: reliable reproducer

[Finn Thain <fthain@linux-m68k.org>]

On Tue, 25 Apr 2023, Andreas Schwab wrote:

> Please try this patch. 

Thanks for looking into this. Your patch solves the problem for me and 
doesn't seem to cause any regression.

Tested-by: Finn Thain <fthain@linux-m68k.org>

> Signal delivery should only happen at insn boundaries, but due to the 
> way the 030 handles return from bus error

The Programmer's Reference Manual says that the format 0xB frame is used 
by '020 as well which is consistent with the ifdef below, so it probably 
should be mentioned in the commit log too. (And I assume that '020 is also 
affected by this bug.)

> 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

We might avoid the word "interrupted" like so:

	| deliver no signal if the fault occurred while insn was 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;

The build uses -Wimplicit-fallthrough so I added this:

	  fallthrough;

> +	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;
>  }
>