Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190402112813.193378-1-glider@google.com> <4c6a1e592fd345618ef7b7d5bee592da@AcuMS.aculab.com>
 <CAG_fn=XA3e+2+DdAN4_Yvw_yBeokVcnW5Cb5-Q0Hs5DbhVxzHw@mail.gmail.com>
In-Reply-To: <CAG_fn=XA3e+2+DdAN4_Yvw_yBeokVcnW5Cb5-Q0Hs5DbhVxzHw@mail.gmail.com>
From:   Alexander Potapenko <glider@google.com>
Date:   Tue, 2 Apr 2019 14:37:30 +0200
Message-ID: <CAG_fn=UiWCEHUaA71nozUbY7G8g7ZH0ni8Lut=hKVr_rNNC3pw@mail.gmail.com>
Subject: Re: [PATCH v2] x86/asm: fix assembly constraints in bitops
To:     David Laight <David.Laight@aculab.com>
Cc:     "paulmck@linux.ibm.com" <paulmck@linux.ibm.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "dvyukov@google.com" <dvyukov@google.com>,
        "jyknight@google.com" <jyknight@google.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "mingo@redhat.com" <mingo@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Apr 2, 2019 at 2:35 PM Alexander Potapenko <glider@google.com> wrot=
e:
>
> On Tue, Apr 2, 2019 at 1:44 PM David Laight <David.Laight@aculab.com> wro=
te:
> >
> > 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 a=
n
> > > 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 a=
rm64
> > 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 =3D 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 on=
e.
>
> > 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, M=
K1 1PT, UK
> > Registration No: 1397386 (Wales)
>
>
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Stra=C3=9Fe, 33
> 80636 M=C3=BCnchen
>
> Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg


--=20
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Stra=C3=9Fe, 33
80636 M=C3=BCnchen

Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg