Messages in this thread | | | From | Alexander Potapenko <> | Date | Tue, 2 Apr 2019 14:37:30 +0200 | Subject | Re: [PATCH v2] x86/asm: fix assembly constraints in bitops |
| |
On Tue, Apr 2, 2019 at 2:35 PM Alexander Potapenko <glider@google.com> wrote: > > On Tue, Apr 2, 2019 at 1:44 PM David Laight <David.Laight@aculab.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]); (let it be bitmap[size-2] so that we don't overflow the buffer) > } > > 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
-- 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
| |