lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 29 Mar 2017 12:32:51 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Doug Berger <opendmb@...il.com>, catalin.marinas@....com,
        will.deacon@....com
Cc:     robh+dt@...nel.org, computersforpeace@...il.com,
        gregory.0xf0@...il.com, f.fainelli@...il.com,
        bcm-kernel-feedback-list@...adcom.com, wangkefeng.wang@...wei.com,
        james.morse@....com, mingo@...nel.org, sandeepa.s.prabhu@...il.com,
        shijie.huang@....com, linus.walleij@...aro.org, treding@...dia.com,
        jonathanh@...dia.com, olof@...om.net, mirza.krak@...il.com,
        suzuki.poulose@....com, bgolaszewski@...libre.com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 1/8] arm64: mm: Allow installation of memory abort
 handlers

Hi,

On Tue, Mar 28, 2017 at 02:34:24PM -0700, Doug Berger wrote:
> From: Florian Fainelli <f.fainelli@...il.com>
> 
> Similarly to what the ARM/Linux kernel provides, add a hook_fault_code()
> function which allows drivers or other parts of the kernel to install
> custom memory abort handlers. This is useful when a given SoC's busing
> does not propagate the exact faulting physical address, but there is a
> way to read it through e.g: a special arbiter driver.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>

Personally, I do not think that it makes sense to allow arbitrary code
to hook such low-level fault handling.

IMO, if it is truly necessary to allow drivers to handle particular
faults, that should be driven by data associated with the relevant
mapping (e.g. the VMA), rather than allowing code to hook *all* faults.

>From my PoV, NAK to this interface to take over low-level fault
handling.

Catalin and Will have the final say here, as the arm64 maintainers.

Thanks,
Mark.

> ---
>  arch/arm64/include/asm/system_misc.h |  3 +++
>  arch/arm64/mm/fault.c                | 15 ++++++++++++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
> index bc812435bc76..e05f5b8c7c1c 100644
> --- a/arch/arm64/include/asm/system_misc.h
> +++ b/arch/arm64/include/asm/system_misc.h
> @@ -38,6 +38,9 @@ void arm64_notify_die(const char *str, struct pt_regs *regs,
>  void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int,
>  					     struct pt_regs *),
>  			   int sig, int code, const char *name);
> +void hook_fault_code(int nr, int (*fn)(unsigned long, unsigned int,
> +				       struct pt_regs *),
> +		     int sig, int code, const char *name);
>  
>  struct mm_struct;
>  extern void show_pte(struct mm_struct *mm, unsigned long addr);
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 4bf899fb451b..cdf1260f1005 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -488,7 +488,7 @@ static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
>  	return 1;
>  }
>  
> -static const struct fault_info {
> +static struct fault_info {
>  	int	(*fn)(unsigned long addr, unsigned int esr, struct pt_regs *regs);
>  	int	sig;
>  	int	code;
> @@ -560,6 +560,19 @@ static const struct fault_info {
>  	{ do_bad,		SIGBUS,  0,		"unknown 63"			},
>  };
>  
> +void __init hook_fault_code(int nr,
> +			    int (*fn)(unsigned long, unsigned int, struct pt_regs *),
> +			    int sig, int code, const char *name)
> +{
> +	BUG_ON(nr < 0 || nr >= ARRAY_SIZE(fault_info));
> +
> +	fault_info[nr].fn	= fn;
> +	fault_info[nr].sig	= sig;
> +	fault_info[nr].code	= code;
> +	fault_info[nr].name	= name;
> +}
> +
> +
>  static const char *fault_name(unsigned int esr)
>  {
>  	const struct fault_info *inf = fault_info + (esr & 63);
> -- 
> 2.12.0
> 

Powered by blists - more mailing lists