Re: [PATCH v4 0/6] lib/base64: add generic encoder/decoder, migrate users

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Mon, Nov 03, 2025 at 10:32:55PM +0000, David Laight wrote:
> On Mon, 3 Nov 2025 21:37:17 +0200
> Andy Shevchenko <andriy.shevchenko@intel.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@intel.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.

No, if it's a generated via a helper script.

> > > > > 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.

You can check a Git history for that.

-- 
With Best Regards,
Andy Shevchenko