On Tue, 15 Jul 2025 at 01:00, Jackson Donaldson <jackson88...@gmail.com> wrote:
>
> v5:
>
> > > + AES_KEY key;
> > > + if ((s->ctrl & TYPE) == 0) {
> > > + AES_set_encrypt_key(keydata, keylen, &key);
> > > + AES_set_decrypt_key(keydata, keylen, &s->internal_key);
> > > + AES_encrypt(s->data, s->result, &key);
> > Here we call AES_set_encrypt_key() and AES_set_decrypt_key()
> > before calling AES_encrypt()...
>
> > > + s->result_index = 16;
> > > + } else if ((s->ctrl & TYPE) == 1 << 8) {
> > > + AES_set_decrypt_key(keydata, keylen, &key);
> > > + AES_set_decrypt_key(keydata, keylen, &s->internal_key);
> > > + AES_decrypt(s->data, s->result, &key);
> >
> > ...here we call AES_set_decrypt_key() twice before
> > calling AES_decrypt(). This looks a bit odd: should we either
> > (a) call both AES_set_decrypt_key() and AES_set_encrypt_key()
> > in each half of the if(), or (b) call AES_set_encyrypt_key()
> > twice in the AES_encrypt() code path ?
> >
> > (Coverity is sometimes wrong, as it's only using a heuristic
> > here, so the other option is "the code as written is correct",
> > but in that case a comment might be helpful for human readers.)
>
> > thanks
> > -- PMM
>
> The AES engine stores an internal key which it uses only for decryption
> when in the last TYPE. This results in the odd-looking call pairs.
> I've added the requested comment.
Your patches are already upstream, so rather than sending
a new version of the whole series, please rebase on
current head-of-git and just send a patch that adds the comment.
thnaks
-- PMM
