On Fri, 9 Feb 2024 at 14:07, Daniel P. Berrangé <[email protected]> wrote:
>
> From: Hyman Huang <[email protected]>
>
> Introduce the SM4 cipher algorithms (OSCCA GB/T 32907-2016).
>
> SM4 (GBT.32907-2016) is a cryptographic standard issued by the
> Organization of State Commercial Administration of China (OSCCA)
> as an authorized cryptographic algorithms for the use within China.
>
> Detect the SM4 cipher algorithms and enable the feature silently
> if it is available.
>
> Signed-off-by: Hyman Huang <[email protected]>
> Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> Reviewed-by: Daniel P. Berrangé <[email protected]>
> Signed-off-by: Daniel P. Berrangé <[email protected]>
Hi; Coverity points out an issue in this code (CID 1546884):
> @@ -701,6 +731,25 @@ static QCryptoCipher
> *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>
> return &ctx->base;
> }
> +#ifdef CONFIG_CRYPTO_SM4
> + case QCRYPTO_CIPHER_ALG_SM4:
> + {
> + QCryptoNettleSm4 *ctx = g_new0(QCryptoNettleSm4, 1);
Here we allocate memory...
> +
> + switch (mode) {
> + case QCRYPTO_CIPHER_MODE_ECB:
> + ctx->base.driver = &qcrypto_nettle_sm4_driver_ecb;
> + break;
> + default:
> + goto bad_cipher_mode;
...but here we may jump to bad_cipher_mode, resulting in
the memory being leaked, because we never free it.
> + }
> +
This could be fixed by not doing the g_new0() until this
point when we know we aren't going to fail. This is what we
do for the other cipher cases in this file that have a switch
where the default for mode is a jump to bad_cipher_mode.
(PS: is it intentional that for some of these ciphers
an unknown mode value is an error returned to the caller
via bad_cipher_mode, and for others it is a g_assert_not_reached()?)
> + sm4_set_encrypt_key(&ctx->key[0], key);
> + sm4_set_decrypt_key(&ctx->key[1], key);
> +
> + return &ctx->base;
> + }
> +#endif
thanks
-- PMM
