Message-ID: <20251103223255.3de9f9d7@pumpkin>
Date: Mon, 3 Nov 2025 22:32:55 +0000
From: David Laight <david.laight.linux@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Kuan-Wei Chiu <visitorckw@...il.com>, Guan-Chun Wu
 <409411716@....tku.edu.tw>, Andrew Morton <akpm@...ux-foundation.org>,
 ebiggers@...nel.org, tytso@....edu, jaegeuk@...nel.org, xiubli@...hat.com,
 idryomov@...il.com, kbusch@...nel.org, axboe@...nel.dk, hch@....de,
 sagi@...mberg.me, home7438072@...il.com, linux-nvme@...ts.infradead.org,
 linux-fscrypt@...r.kernel.org, ceph-devel@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 0/6] lib/base64: add generic encoder/decoder, migrate
 users

On Mon, 3 Nov 2025 21:37:17 +0200
Andy Shevchenko <andriy.shevchenko@...el.com> wrote:

> On Mon, Nov 03, 2025 at 07:29:08PM +0000, David Laight wrote:
> > On Mon, 3 Nov 2025 20:16:46 +0200
> > Andy Shevchenko <andriy.shevchenko@...el.com> wrote:  
> > > On Mon, Nov 03, 2025 at 04:41:41PM +0200, Andy Shevchenko wrote:  
> > > > On Mon, Nov 03, 2025 at 01:22:13PM +0000, David Laight wrote:    
> 
> ...
> 
> > > > Pragma will be hated.  
> > 
> > They have been used in a few other places.
> > and to disable more 'useful' warnings.  
> 
> You can go with pragma, but even though it just hides the potential issues.
> Not my choice.

In this case you really want the version that has '[ 0 .. 255 ] = -1,',
everything else is unreadable and difficult to easily verify.

> 
> > > > I believe there is a better way to do what you want. Let me cook a PoC.    
> > > 
> > > I tried locally several approaches and the best I can come up with is the pre-generated
> > > (via Python script) pieces of C code that we can copy'n'paste instead of that shortened
> > > form. So basically having a full 256 tables in the code is my suggestion to fix the build
> > > issue. Alternatively we can generate that at run-time (on the first run) in
> > > the similar way how prime_numbers.c does. The downside of such an approach is loosing
> > > the const specifier, which I consider kinda important.
> > > 
> > > Btw, in the future here might be also the side-channel attack concerns appear, which would
> > > require to reconsider the whole algo to get it constant-time execution.  
> > 
> > The array lookup version is 'reasonably' time constant.  
> 
> The array doesn't fit the cacheline.

Ignoring all the error characters it is 2 (64 byte) cache lines (if aligned
on a 32 byte boundary).
They'll both be resident for any sane input, I doubt an attacker can determine
when the second one is loaded.
In any case you can load both at the start just to make sure.

> 
> > One option is to offset all the array entries by 1 and subtract 1 after reading the entry.  
> 
> Yes, I was thinking of it, but found a bit weird.
> 
> > That means that the 'error' characters have zero in the array (not -1).
> > At least the compiler won't error that!
> > The extra 'subtract 1' is probably just measurable.  
> 
> > But I'd consider raising a bug on gcc :-)  
> 
> And clang? :-)

clang is probably easier to get fixed.
The warning can be disabled for 'old' compilers - only one build 'tool'
needs to detect errors.

One solution is to disable the warnings in the compilers, but get sparse
(which I think is easier to change?) to do a sane check that allows
the entire array to default to non-zero while still checking for
other errors.

> > One of the uses of ranged designated initialisers for arrays is to change the
> > default value - as been done here.
> > It shouldn't cause a warning.  
> 
> This is prone to mistakes when it's not the default rewrite. I fixed already
> twice such an issue in drivers/hid in the past few months.

I was thinking that if the first initialiser is [ low ... high ] = value
then it should be valid to change any value.
I'm not sure what you fixed, clearly [ 4 ] = 5, [ 4 ] = 6, is an error,
but it might be sane to allow any update of a 'range' initialiser.

	David

Powered by blists - more mailing lists