linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Mikulas Patocka' <mpatocka@redhat.com>
Cc: 'Linus Torvalds' <torvalds@linux-foundation.org>,
	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>
Subject: RE: [PATCH] hex2bin: make the function hex_to_bin constant-time
Date: Mon, 25 Apr 2022 12:59:23 +0000	[thread overview]
Message-ID: <e8de034196df450cb352fa60a570acca@AcuMS.aculab.com> (raw)
In-Reply-To: <alpine.LRH.2.02.2204250701410.10912@file01.intranet.prod.int.rdu2.redhat.com>

From: Mikulas Patocka
> Sent: 25 April 2022 12:04
> 
> On Mon, 25 Apr 2022, David Laight wrote:
> 
> > From: Linus Torvalds
> > > Sent: 24 April 2022 22:42
> > >
> > > On Sun, Apr 24, 2022 at 2:37 PM Linus Torvalds
> > > <torvalds@linux-foundation.org> wrote:
> > > >
> > > > Finally, for the same reason - please don't use ">> 8".  Because I do
> > > > not believe that bit 8 is well-defined in your arithmetic. The *sign*
> > > > bit will be, but I'm not convinced bit 8 is.
> > >
> > > Hmm.. I think it's ok. It can indeed overflow in 'char' and change the
> > > sign in bit #7, but I suspect bit #8 is always fine.
> > >
> > > Still, If you want to just extend the sign bit, ">> 31" _is_ the
> > > obvious thing to use (yeah, yeah, properly "sizeof(int)*8-1" or
> > > whatever, you get my drift).
> >
> > Except that right shifts of signed values are UB.
> > In particular it has always been valid to do an unsigned
> > shift right on a 2's compliment negative number.
> >
> > 	David
> 
> Yes. All the standard versions (C89, C99, C11, C2X) say that right shift
> of a negative value is implementation-defined.
> 
> So, we should cast it to "unsigned" before shifting it.

Except that the intent appears to be to replicate the sign bit.

If it is 'implementation defined' (rather than suddenly being UB)
it might be that the linux kernel requires sign propagating
right shifts of negative values.
This is typically what happens on 2's compliment systems.
But not all small cpu have the required shift instruction.
OTOH all the ones bit enough to run Linux probably do.
(And gcc doesn't support '1's compliment' or 'sign overpunch' cpus.)

The problem is that the compiler writers seem to be entering
a mindset where they are optimising code based on UB behaviour.
So given:
void foo(int x)
{
	if (x >> 1 < 0)
		return;
	do_something();
}
they decide the test is UB, so can always be assumed to be true
and thus do_something() is compiled away.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


  reply	other threads:[~2022-04-25 12:59 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-24 20:54 [PATCH] hex2bin: make the function hex_to_bin constant-time Mikulas Patocka
2022-04-24 21:30 ` Joe Perches
2022-04-24 21:37 ` Linus Torvalds
2022-04-24 21:42   ` Linus Torvalds
2022-04-25  9:37     ` David Laight
2022-04-25 11:04       ` Mikulas Patocka
2022-04-25 12:59         ` David Laight [this message]
2022-04-25 13:33           ` Mikulas Patocka
2022-04-25 12:07   ` [PATCH v2] " Mikulas Patocka
2022-04-25 17:53     ` Linus Torvalds
2022-05-04  8:38     ` Stafford Horne
2022-05-04  8:57       ` Mikulas Patocka
2022-05-04  9:20         ` Andy Shevchenko
2022-05-04  9:47           ` Milan Broz
2022-05-04  9:50             ` Jason A. Donenfeld
2022-05-04 11:54           ` Mikulas Patocka
2022-05-04  9:42       ` Jason A. Donenfeld
2022-05-04  9:44         ` Jason A. Donenfeld
2022-05-04  9:57         ` Jason A. Donenfeld
2022-05-04 10:07           ` Andy Shevchenko
2022-05-04 10:15             ` Jason A. Donenfeld
2022-05-04 18:00               ` Linus Torvalds
2022-05-04 19:42                 ` Jason A. Donenfeld
2022-05-04 19:51                   ` Linus Torvalds
2022-05-04 20:00                     ` Linus Torvalds
2022-05-04 20:12                       ` Stafford Horne
2022-05-04 20:26                         ` Linus Torvalds
2022-05-04 21:24                           ` Linus Torvalds
2022-05-04 19:57                   ` Stafford Horne
2022-05-04 20:10                     ` Linus Torvalds
2022-05-04 20:38                       ` Stafford Horne
2022-05-08  0:37                         ` Stafford Horne
2022-05-11 12:17                           ` Stafford Horne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e8de034196df450cb352fa60a570acca@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=andy@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dm-devel@redhat.com \
    --cc=gmazyland@gmail.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=msnitzer@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).