Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <alpine.LRH.2.02.2204241648270.17244@file01.intranet.prod.int.rdu2.redhat.com>
 <CAHk-=wh+Z+OKH3jRttWGHbWSQq2wVMtdnA=ntDiadZu=VxAC7w@mail.gmail.com>
 <alpine.LRH.2.02.2204250723120.26714@file01.intranet.prod.int.rdu2.redhat.com>
 <YnI7hE4cIfjsdKSF@antec> <YnJI4Ru0AlUgrr9C@zx2c4.com> <YnJOCbLtdATzC+jn@zx2c4.com>
 <YnJQXr3igEMTqY3+@smile.fi.intel.com> <YnJSQ3jJyvhmIstD@zx2c4.com>
 <CAHk-=wgb_eBdjM_mzEvXfRG2EhrSK5MHNGyAj7=4vxvN4U9Rug@mail.gmail.com> <CAHmME9q_-nfGxp8_VCqaritm4N8v8g67AzRjXs9du846JhhpoQ@mail.gmail.com>
In-Reply-To: <CAHmME9q_-nfGxp8_VCqaritm4N8v8g67AzRjXs9du846JhhpoQ@mail.gmail.com>
From:   Stafford Horne <shorne@gmail.com>
Date:   Thu, 5 May 2022 04:57:50 +0900
Message-ID: <CAAfxs77yaLvWx9KnkDZX7E1eDm9N-NVJn5n8=mCK9BU-cSob=A@mail.gmail.com>
Subject: Re: [PATCH v2] hex2bin: make the function hex_to_bin constant-time
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@intel.com>,
        Mikulas Patocka <mpatocka@redhat.com>,
        Andy Shevchenko <andy@kernel.org>,
        device-mapper development <dm-devel@redhat.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        Mike Snitzer <msnitzer@redhat.com>,
        Mimi Zohar <zohar@linux.ibm.com>,
        Milan Broz <gmazyland@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, May 5, 2022 at 4:43 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Linus,
>
> On Wed, May 4, 2022 at 8:00 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Wed, May 4, 2022 at 3:15 AM Jason A. Donenfeld <Jason@zx2c4.com> wro=
te:
> > >
> > > > Alignment? Compiler bug? HW issue?
> > >
> > > Probably one of those, yea. Removing the instruction addresses, the o=
nly
> > > difference between the two compiles is: https://xn--4db.cc/Rrn8usaX/d=
iff#line-440
> >
> > Well, that address doesn't work for me at all. It turns into =D7=90.cc.
> >
> > I'd love to see the compiler problem, since I find those fascinating
> > (mainly because they scare the hell out of me), but those web
> > addresses you use are not working for me.
>
> =D7=90.cc is correct. If you can't load it, your browser or something in
> your stack is broken. Choosing a non-ASCII domain like that clearly a
> bad decision because people with broken stacks can't load it? Yea,
> maybe. But maybe it's like the arch/alpha/ reordering of dependent
> loads applied to the web... A bit of stretch.

I have uploaded a diff I created here:
  https://gist.github.com/54334556f2907104cd12374872a0597c

It shows the same output.

> > It most definitely looks like an OpenRISC compiler bug - that code
> > doesn't look like it does anything remotely undefined (and with the
> > "unsigned char", nothing implementation-defined either).
>
> I'm not so certain it's in the compiler anymore, actually. The bug
> exhibits itself even when that code isn't actually called. Adding nops
> to unrelated code also makes the problem go away. And removing these
> nops [1] makes the problem go away too. So maybe it's looking more
> like a linker bug (or linker script bug) related to alignment. Or
> whatever is jumping between contexts in the preemption code and
> restoring registers and such is assuming certain things about code
> layout that doesn't always hold. More fiddling is necessary still.

Bisecting definitely came to this patch which is strange. Then reverting
e5be15767e7e ("hex2bin: make the function hex_to_bin constant-time")
did also fix the problem for me.

But it could be any small patch that changes layout could make this go away=
.

I have things to try:
  - more close look at the produced asembly diff
  - newer compiler (I fixed a few bugs in gcc 12 for openrisc, and
this testing came up in gcc 11)
  - trying on FPGA's

I'll report as I find things.

-Stafford