On Tue, 12 Mar 2024 at 16:03, Chang S. Bae <chang.seok.bae@xxxxxxxxx> wrote: > > On 3/12/2024 12:46 AM, Ard Biesheuvel wrote: > > On Mon, 11 Mar 2024 at 22:48, Chang S. Bae <chang.seok.bae@xxxxxxxxx> wrote: > >> > >> @@ -241,7 +241,7 @@ static int aes_set_key_common(struct crypto_aes_ctx *ctx, > >> err = aes_expandkey(ctx, in_key, key_len); > >> else { > >> kernel_fpu_begin(); > >> - err = aesni_set_key(ctx, in_key, key_len); > >> + aesni_set_key(ctx, in_key, key_len); > > > > This will leave 'err' uninitialized. > > Ah, right. Thanks for catching it. > > Also, upon reviewing aes_expandkey(), I noticed there's no error case, > except for the key length sanity check. > > While addressing this, perhaps additional cleanup is considerable like: > > @@ -233,19 +233,20 @@ static int aes_set_key_common(struct > crypto_aes_ctx *ctx, > { > int err; > > - if (key_len != AES_KEYSIZE_128 && key_len != AES_KEYSIZE_192 && > - key_len != AES_KEYSIZE_256) > - return -EINVAL; > + err = aes_check_keylen(key_len); > + if (err) > + return err; > > if (!crypto_simd_usable()) > - err = aes_expandkey(ctx, in_key, key_len); > + /* no error with a valid key length */ > + aes_expandkey(ctx, in_key, key_len); > else { > kernel_fpu_begin(); > - err = aesni_set_key(ctx, in_key, key_len); > + aesni_set_key(ctx, in_key, key_len); > kernel_fpu_end(); > } > > - return err; > + return 0; > } > I wonder whether we need aesni_set_key() at all.