Skip to contentSkip to conversation

crypto: qat - Add support for RSA algorithm

DISCUSSION Dan Carpenter 2 participants 4 messages Jul 20, 2015 Crypto API

lore
Share & tools
lore
#0 lore raw

Hello Tadeusz Struk,

The patch a990532023b9: "crypto: qat - Add support for RSA algorithm" from Jul 15, 2015, leads to the following Smatch warning:

	drivers/crypto/qat/qat_common/qat_asym_algs.c:446 qat_rsa_get_n()
	warn: was && intended here instead of ||?

drivers/crypto/qat/qat_common/qat_asym_algs.c

   444          ret = -EINVAL;
   445          /* In FIPS mode only allow key size 2K & 3K */
   446          if (fips_enabled && (ctx->key_sz != 256 || ctx->key_sz != 384)) {
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Looks like the static checker is correct.

   447                  pr_err("QAT: RSA: key size not allowed in FIPS mode\n");
   448                  goto err;
   449          }
   450          /* invalid key size provided */
   451          if (!qat_rsa_enc_fn_id(ctx->key_sz))
   452                  goto err;
   453  
   454          ret = -ENOMEM;
   455          ctx->n = dma_zalloc_coherent(dev, ctx->key_sz, &ctx->dma_n, GFP_KERNEL);
   456          if (!ctx->n)
   457                  goto err;
   458  
   459          memcpy(ctx->n, ptr, ctx->key_sz);
   460          return 0;
   461  err:
   462          ctx->key_sz = 0;
   463          ctx->n = NULL;
   464          return ret;
   465  }

regards,
dan carpenter

#1 lore raw

Also:

	drivers/crypto/qat/qat_common/qat_asym_algs.c:513 qat_rsa_get_d()
	warn: was && intended here instead of ||?

regards,
dan carpenter

#2 lore raw
In reply to Dan Carpenter
Quoted 6 lines · Dan Carpenter

The patch a990532023b9: "crypto: qat - Add support for RSA algorithm" from Jul 15, 2015, leads to the following Smatch warning:

	drivers/crypto/qat/qat_common/qat_asym_algs.c:446 qat_rsa_get_n()
	warn: was && intended here instead of ||?
Quoted 7 lines

drivers/crypto/qat/qat_common/qat_asym_algs.c

   444          ret = -EINVAL;
   445          /* In FIPS mode only allow key size 2K & 3K */
   446          if (fips_enabled && (ctx->key_sz != 256 || ctx->key_sz != 384)) {
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Looks like the static checker is correct.

The logic is if fips_enabled we want to accept only key size 2K (256 bytes) or 3K (384 bytes) so the condition looks ok to me. Maybe the comment above is misleading? regards,
T

#3 lore raw
Quoted 13 lines · Tadeusz Struk
Quoted 12 lines · Dan Carpenter

The patch a990532023b9: "crypto: qat - Add support for RSA algorithm" from Jul 15, 2015, leads to the following Smatch warning:

	drivers/crypto/qat/qat_common/qat_asym_algs.c:446 qat_rsa_get_n()
	warn: was && intended here instead of ||?

drivers/crypto/qat/qat_common/qat_asym_algs.c

   444          ret = -EINVAL;
   445          /* In FIPS mode only allow key size 2K & 3K */
   446          if (fips_enabled && (ctx->key_sz != 256 || ctx->key_sz != 384)) {
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoted 4 lines
Quoted 1 line

Looks like the static checker is correct.

The logic is if fips_enabled we want to accept only key size 2K (256 bytes) or 3K (384 bytes) so the condition looks ok to me. Maybe the comment above is misleading?

Logically ->key_sz can't be both 256 and 384 at the same time. What you are describing is &&. This seems like an important function, my guess is we have never tested it with fips_enabled?

regards,
dan carpenter