linux-kernel - Re: [PATCH v2] x86/asm: fix assembly constraints in bitops

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: <CAG_fn=XA3e+2+DdAN4_Yvw_yBeokVcnW5Cb5-Q0Hs5DbhVxzHw@mail.gmail.com>
Date:   Tue, 2 Apr 2019 14:35:58 +0200
From:   Alexander Potapenko <glider@...gle.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     "paulmck@...ux.ibm.com" <paulmck@...ux.ibm.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dvyukov@...gle.com" <dvyukov@...gle.com>,
        "jyknight@...gle.com" <jyknight@...gle.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>
Subject: Re: [PATCH v2] x86/asm: fix assembly constraints in bitops

On Tue, Apr 2, 2019 at 1:44 PM David Laight <David.Laight@...lab.com> wrote:
>
> From: Alexander Potapenko
> > Sent: 02 April 2019 12:28
> >
> > 1. Use memory clobber in bitops that touch arbitrary memory
> >
> > Certain bit operations that read/write bits take a base pointer and an
> > arbitrarily large offset to address the bit relative to that base.
>
> Although x86_64 can use a signed 64bit bit number, looking at arm and arm64
> they use 'int nr' throughout as do the generic functions.
> Maybe x86 ought to be consistent here.
> I doubt negative bit numbers are expected to work?
I don't have a strong opinion on this, but the corresponding Intel
instructions do accept 64-bit operands.

> Did you try telling gcc that a big buffer (250MB is the limit for 32bit)
> from the pointer might be changed?
Yes, I did, see
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1966993.html
This still isn't a silver bullet, e.g. I saw an example where touching
a function parameter cast to a big buffer in the assembly resulted in
clobbering a global.
Moreover, one can imagine a situation where such a trick may be harmful, e.g.:

void foo(int size) {
  struct arr {
    long val[1U<<28];
  };
  long *bitmap = malloc(size);
  asm("#do something" : "+m"(*(struct arr*)bitmap);
  if (size < 1024)
    process(bitmap[size]);
}

If a (smart enough) compiler knows that malloc(size) returns a pointer
to |size| bytes in memory, it may assume that |size| is at least
1U<<28 (because otherwise it's incorrect to treat |bitmap| as a
pointer to a big array) and delete the size check.
This is of course a synthetic example, but not a completely impossible one.

> That ought to be softer than a full 'memory' clobber as it should
> only affect memory that could be accessed through the pointer.
>
> ....
> > -#define BITOP_ADDR(x) "+m" (*(volatile long *) (x))
> > +#define RLONG_ADDR(x) "m" (*(volatile long *) (x))
> > +#define WBYTE_ADDR(x) "+m" (*(volatile char *) (x))
> >
> > -#define ADDR                         BITOP_ADDR(addr)
> > +#define ADDR                         RLONG_ADDR(addr)
>
> Is it worth just killing ADDR ?
> (as a different patch)
Agreed.
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg