On Fri, 4 Jul 2025 at 23:32, Jackson Donaldson <jackson88...@gmail.com> wrote: > > This commit implements AES for the MAX78000 > > Signed-off-by: Jackson Donaldson <jc...@duck.com> > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
Hi; now this is upstream Coverity Scan noticed a possible issue in this function (CID 1612247): > +static void max78000_aes_do_crypto(Max78000AesState *s) > +{ > + int keylen = 256; > + uint8_t *keydata = s->key; > + if ((s->ctrl & KEY_SIZE) == 0) { > + keylen = 128; > + keydata += 16; > + } else if ((s->ctrl & KEY_SIZE) == 1 << 6) { > + keylen = 192; > + keydata += 8; > + } > + > + 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