[PATCH v8] arm64: Expose FAR_EL1 tag bits in siginfo

[Eric W. Biederman]

Peter Collingbourne <pcc at google.com> writes:

> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 47f651df781c..a8380a2b6361 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -235,20 +235,41 @@ static void arm64_show_signal(int signo, const char *str)
>  }
>  
>  void arm64_force_sig_fault(int signo, int code, void __user *addr,
> +			   unsigned long far, unsigned char far_tb_mask,
>  			   const char *str)
>  {
>  	arm64_show_signal(signo, str);
> -	if (signo == SIGKILL)
> +	if (signo == SIGKILL) {
>  		force_sig(SIGKILL);
> -	else
> -		force_sig_fault(signo, code, addr);
> +	} else {
> +		struct kernel_siginfo info;
> +		clear_siginfo(&info);
> +		info.si_signo = signo;
> +		info.si_errno = 0;
> +		info.si_code = code;
> +		info.si_addr = addr;
> +		info.si_addr_top_byte = (far >> 56) & far_tb_mask;
> +		info.si_addr_top_byte_mask = far_tb_mask;
> +		force_sig_info(&info);
> +	}
>  }
>  
>  void arm64_force_sig_mceerr(int code, void __user *addr, short lsb,
> -			    const char *str)
> +			    unsigned long far, const char *str)
>  {
> +	struct kernel_siginfo info;
> +
>  	arm64_show_signal(SIGBUS, str);
> -	force_sig_mceerr(code, addr, lsb);
> +
> +	clear_siginfo(&info);
> +	info.si_signo = SIGBUS;
> +	info.si_errno = 0;
> +	info.si_code = code;
> +	info.si_addr = addr;
> +	info.si_addr_lsb = lsb;
> +	info.si_addr_top_byte = far >> 56;
> +	info.si_addr_top_byte_mask = 0xff;
> +	force_sig_info(&info);
>  }

I have a real problem with this construction.  force_sig_info is not an
interface that should be used for anything except to define a wrapper
that takes it's parameters.

It is not clear to me that if you have adapted siginfo_layout.


> diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
> index cb3d6c267181..6dd82373eb2d 100644
> --- a/include/uapi/asm-generic/siginfo.h
> +++ b/include/uapi/asm-generic/siginfo.h
> @@ -91,6 +91,14 @@ union __sifields {
>  				char _dummy_pkey[__ADDR_BND_PKEY_PAD];
>  				__u32 _pkey;
>  			} _addr_pkey;
> +#ifdef __aarch64__
> +			/* used with all si_codes */
> +			struct {
> +				short _dummy_top_byte;
> +				unsigned char _top_byte;
> +				unsigned char _top_byte_mask;
> +			} _addr_top_byte;
> +#endif
>  		};
>  	} _sigfault;
>

Why the _dummy_top_byte?  Oh I see it should be spelled "short _addr_lsb;".

Please remove the "#ifdef __aarch64__".  If at all possible we want to
design this so any other architecture who has this challenge can use the
code.  The kind of code does not get enough attention/maintenance if it
is built for a single architecture.


> @@ -148,6 +156,10 @@ typedef struct siginfo {
>  #define si_int		_sifields._rt._sigval.sival_int
>  #define si_ptr		_sifields._rt._sigval.sival_ptr
>  #define si_addr		_sifields._sigfault._addr
> +#ifdef __aarch64__
> +#define si_addr_top_byte	_sifields._sigfault._addr_top_byte._top_byte
> +#define si_addr_top_byte_mask	_sifields._sigfault._addr_top_byte._top_byte_mask
> +#endif
>  #ifdef __ARCH_SI_TRAPNO
>  #define si_trapno	_sifields._sigfault._trapno
>  #endif


The details asside I think this set of changes is making progress.

Eric