linux-kernel - Re: [PATCH] ARCv2: Don't pretend we may set U & DE bits in STATUS32 with kflag

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] [day] [month] [year] [list]

Date:   Wed, 17 Jul 2019 18:35:02 +0000
From:   Vineet Gupta <Vineet.Gupta1@...opsys.com>
To:     Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
        "linux-snps-arc@...ts.infradead.org" 
        <linux-snps-arc@...ts.infradead.org>
CC:     Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARCv2: Don't pretend we may set U & DE bits in STATUS32
 with kflag

On 7/16/19 1:51 PM, Alexey Brodkin wrote:
> As per PRM "kflag" instruction doesn't change state of
> DE-flag ("Delayed branch is pending") and U-flag ("User mode")
> in STATUS32 register so let's not act as if we can affect those bits.

I understand the motivation and indeed bits not writable by kflag should be
removed. But what if we do need to clear those bits out from status32 (assuming
they exist there in first place) and that kflag might not be the right instruction
to do that.
So the question to ask is do we need to clear U and DE bits from status32 and
answer from reading the PRM is no, we don't have to, as those are cleared already
when an exception is taken.

The likely reason this code is because back in arc700 days we used to have a
different version of this macro which atleast in original code relied on using the
pre-exception status32 (in pt_regs) - and that could easily have U and/or DE set
based hence needed clearing.

.macro FAKE_RET_FROM_EXCPN  reg

    ld  \reg, [sp, PT_status32]
    and \reg, \reg, ~(STATUS_U_MASK|STATUS_DE_MASK)
    or  \reg, \reg, STATUS_L_MASK
    sr  \reg, [erstatus]
    mov \reg, 55f
    sr  \reg, [eret]

    rtie
55:
.endm

This is not needed (even in arc700) if we are using the current "in-exception"
status32 for doing the early return.

Long story short, your patch is correct but we need to explain better why it is.
I've applied it locally with slight tweak to changelog to that effect.

> Signed-off-by: Alexey Brodkin <abrodkin@...opsys.com>
> ---
>  arch/arc/include/asm/entry-arcv2.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arc/include/asm/entry-arcv2.h b/arch/arc/include/asm/entry-arcv2.h
> index 225e7df2d8ed..6558e2edb583 100644
> --- a/arch/arc/include/asm/entry-arcv2.h
> +++ b/arch/arc/include/asm/entry-arcv2.h
> @@ -237,7 +237,7 @@
>  
>  .macro FAKE_RET_FROM_EXCPN
>  	lr      r9, [status32]
> -	bic     r9, r9, (STATUS_U_MASK|STATUS_DE_MASK|STATUS_AE_MASK)
> +	bic     r9, r9, STATUS_AE_MASK
>  	or      r9, r9, STATUS_IE_MASK
>  	kflag   r9
>  .endm