linux-kernel - Re: [RFC][PATCH 07/22] x86,extable: Extend extable functionality

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:   Fri, 5 Nov 2021 20:32:29 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org, jpoimboe@...hat.com,
        mark.rutland@....com, dvyukov@...gle.com, pbonzini@...hat.com,
        mbenes@...e.cz
Subject: Re: [RFC][PATCH 07/22] x86,extable: Extend extable functionality

On Fri, Nov 05, 2021 at 07:17:20PM +0000, Sean Christopherson wrote:
> On Fri, Nov 05, 2021, Peter Zijlstra wrote:
> > On Fri, Nov 05, 2021 at 05:32:14PM +0000, Sean Christopherson wrote:
> > 
> > > > +#define EX_IMM_MASK	0xFFFF0000
> > 
> > > > +	imm  = FIELD_GET(EX_IMM_MASK,  e->type);
> > > 
> > > FIELD_GET casts the result based on the type of the mask, but doesn't explicitly
> > > sign extend the masked field, i.e. there's no intermediate cast to tell the compiler
> > > that the imm is a 16-bit value that should be sign extended.
> > > 
> > > Modifying FIELD_GET to sign extended is probably a bad idea as I'm guessing the
> > > vast, vast majority of use cases don't want that behavior.  I'm not sure how that
> > > would even work with masks that are e.g. 5 bits or so.
> > 
> > So the way I was reading it was that typeof(_mask) is 'int', e->type is
> > also 'int', we mask out the top bits, and since it's all 'int' we do an
> > arith shift right (ie. preserves sign).
> > 
> > Where did that reading go wrong?
> 
> Hmm, C99 standard says that right shift with a negative value is implementation
> specific:

C99 is sodding daft wrt signed values. That's why we force -fwrapv and
say signed is 2s complement and expect sanity.

> gcc-10 generates a bare "shr", i.e. doesn't special case negative values, and "shr"
> is explicitly defined as an unsigned divide.

We hard rely on signed shift right to preserve sign all over the place,
how come it goes sideways here? Lemme go stare at asm...