linux-kernel - Re: [PATCH 3/9] arm64: mm: install SError abort handler

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]

Message-ID: <20170324151654.GD29588@leverpostej>
Date:   Fri, 24 Mar 2017 15:16:54 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Doug Berger <opendmb@...il.com>
Cc:     catalin.marinas@....com, robh+dt@...nel.org, will.deacon@....com,
        computersforpeace@...il.com, gregory.0xf0@...il.com,
        f.fainelli@...il.com, bcm-kernel-feedback-list@...adcom.com,
        wangkefeng.wang@...wei.com, james.morse@....com,
        vladimir.murzin@....com, panand@...hat.com, andre.przywara@....com,
        cmetcalf@...lanox.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, horms+renesas@...ge.net.au,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/9] arm64: mm: install SError abort handler

On Fri, Mar 24, 2017 at 07:46:26AM -0700, Doug Berger wrote:
> This commit adds support for minimal handling of SError aborts and
> allows them to be hooked by a driver or other part of the kernel to
> install a custom SError abort handler.  The hook function returns
> the previously registered handler so that handlers may be chained if
> desired.
> 
> The handler should return the value 0 if the error has been handled,
> otherwise the handler should either call the next handler in the
> chain or return a non-zero value.

... so the order these get calls is completely dependent on probe
order...

> Since the Instruction Specific Syndrome value for SError aborts is
> implementation specific the registerred handlers must implement
> their own parsing of the syndrome.

... and drivers have to be intimately familiar with the CPU, in order to
be able to parse its IMPLEMENTATION DEFINED ESR_ELx.ISS value.

Even then, there's no guarantee there's anything useful there, since it
is IMPLEMENTATION DEFINED and could simply be RES0 or UNKNOWN in all
cases.

I do not think it is a good idea to allow arbitrary drivers to hook
this fault in this manner.

> +	.align	6
> +el0_error:
> +	kernel_entry 0
> +el0_error_naked:
> +	mrs	x25, esr_el1			// read the syndrome register
> +	lsr	x24, x25, #ESR_ELx_EC_SHIFT	// exception class
> +	cmp	x24, #ESR_ELx_EC_SERROR		// SError exception in EL0
> +	b.ne	el0_error_inv
> +el0_serr:
> +	mrs	x26, far_el1
> +	// enable interrupts before calling the main handler
> +	enable_dbg_and_irq

... why?

We don't do this for inv_entry today.

> +	ct_user_exit
> +	bic	x0, x26, #(0xff << 56)
> +	mov	x1, x25
> +	mov	x2, sp
> +	bl	do_serr_abort
> +	b	ret_to_user
> +el0_error_inv:
> +	enable_dbg
> +	mov	x0, sp
> +	mov	x1, #BAD_ERROR
> +	mov	x2, x25
> +	b	bad_mode
> +ENDPROC(el0_error)

Clearly you expect these to be delivered at arbitrary times during
execution. What if a KVM guest is executing at the time the SError is
delivered?

To be quite frank, I don't believe that we can reliably and safely
handle this misfeature in the kernel, and this infrastructure only
provides the illusion that we can.

I do not think it makes sense to do this.

Thanks,
Mark.