Re: [PATCH 15/15] wifi: mac80211: Use AES-CMAC library in aes_s2v()

[Johannes Berg]

On Thu, 2026-02-19 at 14:15 -0800, Eric Biggers wrote:
> > > -static int aes_s2v(struct crypto_shash *tfm,
> > > +static int aes_s2v(const u8 *in_key, size_t key_len,
> > >  		   size_t num_elem, const u8 *addr[], size_t len[], u8 *v)
> > >  {
> > >  	u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE] = {};
> > > -	SHASH_DESC_ON_STACK(desc, tfm);
> > > +	struct aes_cmac_key key;
> > > +	struct aes_cmac_ctx ctx;
> > >  	size_t i;
> > > +	int res;
> > >  
> > > -	desc->tfm = tfm;
> > > +	res = aes_cmac_preparekey(&key, in_key, key_len);
> > > +	if (res)
> > > +		return res;
> > 
> > Same here, maybe, technically, but also doesn't matter.
> > 
> > Acked-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
> > 
> > johannes
> 
> In this case aes_s2v() wouldn't otherwise be able to fail, so ignoring
> the aes_cmac_preparekey() return value would indeed be a simplification.

Right.

> However, since the key length isn't a compile-time constant here, we'd
> have to rely on non-local validation, which isn't ideal.

That's true.

> To ignore the return value entirely I'd prefer a static_assert that the
> length is equal to one of AES_KEYSIZE_*, which isn't possible here.

Indeed.

> It's actually not clear to me where the length validation happens before
> here.  nl80211_associate() for example just copies the length from
> userspace without validating it.  ieee80211_mgd_assoc() only checks that
> the length is at most FILS_MAX_KEK_LEN (64).

Oh, right, I forgot this was FILS and mixed it up with the previous
patch. We probably _should_ check it earlier there, but it won't be
local here either way, and we have the error paths already so that's
fine.

johannes