linux-kernel - Re: [RESEND,PATCH] ARM: fix __div64_32() error when compiling with clang

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:   Tue, 24 Nov 2020 15:42:34 +0800
From:   Antony Yu <swpenim@...il.com>
To:     Nathan Chancellor <natechancellor@...il.com>
Cc:     Russell King <linux@...linux.org.uk>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        clang-built-linux@...glegroups.com
Subject: Re: [RESEND,PATCH] ARM: fix __div64_32() error when compiling with
 clang

On Mon, Nov 23, 2020 at 11:16:02AM -0700, Nathan Chancellor wrote:
> On Mon, Nov 23, 2020 at 03:36:32PM +0800, Antony Yu wrote:
> > __do_div64 clobbers the input register r0 in little endian system.
> > According to the inline assembly document, if an input operand is
> > modified, it should be tied to a output operand. This patch can
> > prevent compilers from reusing r0 register after asm statements.
> > 
> > Signed-off-by: Antony Yu <swpenim@...il.com>
> > ---
> >  arch/arm/include/asm/div64.h | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/div64.h b/arch/arm/include/asm/div64.h
> > index 898e9c78a7e7..809efc51e90f 100644
> > --- a/arch/arm/include/asm/div64.h
> > +++ b/arch/arm/include/asm/div64.h
> > @@ -39,9 +39,10 @@ static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
> >  	asm(	__asmeq("%0", __xh)
> >  		__asmeq("%1", "r2")
> >  		__asmeq("%2", "r0")
> > -		__asmeq("%3", "r4")
> > +		__asmeq("%3", "r0")
> > +		__asmeq("%4", "r4")
> >  		"bl	__do_div64"
> > -		: "=r" (__rem), "=r" (__res)
> > +		: "=r" (__rem), "=r" (__res), "=r" (__n)
> >  		: "r" (__n), "r" (__base)
> >  		: "ip", "lr", "cc");
> >  	*n = __res;
> > -- 
> > 2.23.0
> > 
> 
> I am not sure that I am qualified to review this (my assembly knowledge
> is not the best) but your commit title mentions an error when compiling
> with clang. What is the exact error, what configuration generates it,
> and what version of clang? We have done fairly decent testing for
> 32-bit ARM, I would like to know what we are missing.
> 
> Cheers,
> Nathan

We have run fail on android R vts vts_libsnapshot_test with kernel 4.14.
This bug is triggered accidently by a workaround patch in our code base.
It is fine on a pure clean 4.14 branch since __do_div64 may not be
executed in skip_metadata.

The attachment are .i and generated .s file. .s file can be reproduced
with clang -target arm-linux-eabi -march=armv8.2-a -O2.

In function skip_metadata, it loads some value to r0, calls __do_div64,
adds 1 to r0 and stores it to [r5]. It gets wrong value since __do_div64
clobbers r0 register.

We have tried clang-10, clang-11 and android prebuilt clang-r383902b. All
of them have the same problem.

View attachment "dm-snap-persistent_clang.trim.i" of type "text/plain" (1373898 bytes)

View attachment "dm-snap-persistent_clang.trim.s" of type "text/plain" (1983 bytes)