On Sat, Dec 21, 2024 at 07:16:07PM +0800, Herbert Xu wrote: > Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > > > @@ -326,13 +326,13 @@ int skcipher_walk_virt(struct skcipher_walk *walk, > > return 0; > > > > scatterwalk_start(&walk->in, req->src); > > scatterwalk_start(&walk->out, req->dst); > > > > - walk->blocksize = crypto_skcipher_blocksize(tfm); > > - walk->ivsize = crypto_skcipher_ivsize(tfm); > > - walk->alignmask = crypto_skcipher_alignmask(tfm); > > + walk->blocksize = alg->base.cra_blocksize; > > + walk->ivsize = alg->co.ivsize; > > + walk->alignmask = alg->base.cra_alignmask; > > Please do this instead: > > unsigned bs, ivs, am; > > bs = crypto_skcipher_blocksize(tfm); > ivs = crypto_skcipher_ivsize(tfm); > am = crypto_skcipher_alignmask(tfm); > walk->blocksize = bs; > walk->ivsize = ivs; > walk->alignmask = am; > > This generates the right thing for me with gcc12. > This seems strictly worse than my version, so I don't plan to do this. It's more lines of code, and it causes an extra push and pop to be needed in skcipher_walk_virt() to free up enough registers to hold all values at once. It may be intended that API users are supposed to use the helper functions instead of accessing the algorithm struct directly, but this code is not a user; it's part of the API implementation in crypto/skcipher.c. There are already lots of other direct accesses to the algorithm struct in the same file, and even another in skcipher_walk_virt() already. The helper functions are pointless in this context and just cause problems like the one this patch is fixing. - Eric
