[intel-linux-intel-lts:5.4/yocto 13/1142] drivers/crypto/keembay/keembay-ocs-ecc-core.c:659 kmb_ocs_ecdh_set_secret() error: we previously assumed 'params.key' could be null (see line 642)

All of lore.kernel.org
 help / color / mirror / Atom feed

From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org
Subject: [intel-linux-intel-lts:5.4/yocto 13/1142] drivers/crypto/keembay/keembay-ocs-ecc-core.c:659 kmb_ocs_ecdh_set_secret() error: we previously assumed 'params.key' could be null (see line 642)
Date: Fri, 11 Dec 2020 11:13:52 +0300	[thread overview]
Message-ID: <20201211081352.GP2767@kadam> (raw)

[-- Attachment #1: Type: text/plain, Size: 12702 bytes --]

tree:   https://github.com/intel/linux-intel-lts.git 5.4/yocto
head:   eeb611e5394c56d45c5cc8f7dc484c9f19e93143
commit: d2a205db2c4ee2728f9be6d45f2361f05205408e [13/1142] crypto: keembay: Add Keem Bay offload ECC Driver
config: i386-randconfig-m021-20201211 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/crypto/keembay/keembay-ocs-ecc-core.c:659 kmb_ocs_ecdh_set_secret() error: we previously assumed 'params.key' could be null (see line 642)
drivers/crypto/keembay/keembay-ocs-ecc-core.c:680 kmb_ocs_ecc_do_one_request() warn: this array is probably non-NULL. 'ctx->private_key'

vim +659 drivers/crypto/keembay/keembay-ocs-ecc-core.c

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  619  static int kmb_ocs_ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  620  				   unsigned int len)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  621  {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  622  	struct ecdh params;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  623  	unsigned int ndigits;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  624  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  625  	int ret = 0;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  626  	struct ocs_ecc_ctx *ctx = kpp_tfm_ctx(tfm);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  627  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  628  	ret = crypto_ecdh_decode_key(buf, len, &params);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  629  	if (ret)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  630  		goto cleanup;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  631  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  632  	ndigits = kmb_ocs_ecdh_supported_curve(params.curve_id);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  633  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  634  	if (!ndigits) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  635  		ret = -EOPNOTSUPP;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  636  		goto cleanup;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  637  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  638  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  639  	ctx->curve_id = params.curve_id;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  640  	ctx->ndigits = ndigits;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  641  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12 @642  	if (!params.key || !params.key_size) {
                                                             ^^^^^^^^^^
"params.key" is NULL.

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  643  		ret = -EINVAL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  644  #ifdef	CONFIG_CRYPTO_DEV_KEEMBAY_OCS_ECDH_GEN_PRIV_KEY_SUPPORT
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  645  		ret = kmb_ecc_gen_privkey(ctx->curve_id, ctx->ndigits,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  646  					  ctx->private_key);

We allocate a new key, but I don't see where it is assigned to
"params.key".

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  647  #endif  /* CONFIG_CRYPTO_DEV_KEEMBAY_OCS_ECDH_GEN_PRIV_KEY_SUPPORT */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  648  		if (ret)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  649  			goto cleanup;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  650  		goto swap_digits;
                                                                ^^^^^^^^^^^^^^^^^

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  651  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  652  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  653  	ret = kmb_ecc_is_key_valid(ctx->curve_id, ctx->ndigits,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  654  				   (const u64 *)params.key, params.key_size);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  655  	if (ret)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  656  		goto cleanup;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  657  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  658  swap_digits:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12 @659  	ecc_swap_digits((u64 *)params.key, ctx->private_key, ctx->ndigits);
                                                                         ^^^^^^^^^^^^^^^^
NULL dereference.

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  660  cleanup:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  661  	memzero_explicit(&params, sizeof(params));
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  662  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  663  	return ret;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  664  }
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  665  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  666  static int kmb_ocs_ecc_do_one_request(struct crypto_engine *engine,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  667  				      void *areq)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  668  {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  669  	size_t copied, nbytes;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  670  	struct ecc_point *pk;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  671  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  672  	u64 *public_key = NULL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  673  	int ret = -ENOMEM;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  674  	size_t public_key_sz = 0;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  675  	struct kpp_request *req = container_of(areq, struct kpp_request,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  676  					       base);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  677  	struct ocs_ecc_ctx *ctx = kmb_ocs_kpp_ctx(req);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  678  	const struct ecc_curve *curve = ecc_get_curve(ctx->curve_id);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  679  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12 @680  	if (!ctx->private_key || !curve ||
                                                             ^^^^^^^^^^^^^^^^
Delete this check.

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  681  	    (ctx->ndigits > KMB_ECC_MAX_DIGITS)) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  682  		ret = -EINVAL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  683  		goto out;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  684  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  685  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  686  	/* No spurious request checked at top level.*/
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  687  	if ((!req->src) && (!req->dst)) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  688  		ret = -EINVAL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  689  		goto out;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  690  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  691  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  692  	/* Store the request flag in ctx. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  693  	ctx->gfp = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ? GFP_KERNEL
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  694  								: GFP_ATOMIC;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  695  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  696  	pk = ecc_alloc_point(ctx->ndigits, ctx->gfp);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  697  	if (!pk) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  698  		ret = -ENOMEM;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  699  		goto out;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  700  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  701  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  702  	/* Store the kpp_request struct in the device context. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  703  	ctx->ecc_dev->req = req;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  704  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  705  	/* In case shared_secret branch not taken. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  706  	ctx->pk = NULL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  707  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  708  	nbytes = data_size_u64_to_u8(ctx->ndigits);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  709  	/* Public part is a point thus it has both coordinates */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  710  	public_key_sz = 2 * nbytes;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  711  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  712  	public_key = kzalloc(public_key_sz, ctx->gfp);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  713  	if (!public_key) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  714  		ret = -ENOMEM;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  715  		goto free_all;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  716  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  717  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  718  	if (req->src) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  719  		/* from here on it's invalid parameters */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  720  		ret = -EINVAL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  721  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  722  		/* must have exactly two points to be on the curve */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  723  		if (public_key_sz != req->src_len)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  724  			goto free_all;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  725  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  726  		copied = sg_copy_to_buffer(req->src,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  727  					   sg_nents_for_len(req->src,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  728  							    public_key_sz),
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  729  					   public_key, public_key_sz);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  730  		if (copied != public_key_sz)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  731  			goto free_all;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  732  		/* Store pk in the device context. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  733  		ctx->pk = pk;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  734  		ecc_swap_digits(public_key, pk->x, ctx->ndigits);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  735  		ecc_swap_digits(&public_key[ctx->ndigits], pk->y, ctx->ndigits);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  736  		/*
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  737  		 * Check the public key for following
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  738  		 * Check 1: Verify key is not the zero point.
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  739  		 * Check 2: Verify key is in the range [1, p-1].
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  740  		 * Check 3: Verify that y^2 == (x^3 + a·x + b) mod p
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  741  		 */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  742  		ret = kmb_ocs_ecc_is_pubkey_valid_partial(ctx->ecc_dev, curve,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  743  							  pk);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  744  	} else {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  745  		/* Public Key(pk) = priv * G. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  746  		ret = ecc_point_mult(ctx->ecc_dev, pk, &curve->g,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  747  				     ctx->private_key, curve, ctx->ndigits);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  748  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  749  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  750  	if (ret)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  751  		goto free_all;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  752  	goto return_success;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  753  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  754  	/* follow through */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  755  free_all:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  756  	ecc_free_point(pk);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  757  out:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  758  	crypto_finalize_kpp_request(ctx->ecc_dev->engine,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  759  				    req, ret);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  760  return_success:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  761  	if (public_key) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  762  		memzero_explicit(public_key, public_key_sz);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  763  		kzfree(public_key);
                                                                ^^^^^^^^^^^^^^^^^^
I feel like memzero_explicit() and kzfree() are hopefully duplicative.
What is the point of kzfree() if it doesn't have a guaranteed memzero()?

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  764  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  765  	return 0;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  766  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28123 bytes --]

WARNING: multiple messages have this Message-ID (diff)

From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild-all@lists.01.org
Subject: [intel-linux-intel-lts:5.4/yocto 13/1142] drivers/crypto/keembay/keembay-ocs-ecc-core.c:659 kmb_ocs_ecdh_set_secret() error: we previously assumed 'params.key' could be null (see line 642)
Date: Fri, 11 Dec 2020 11:13:52 +0300	[thread overview]
Message-ID: <20201211081352.GP2767@kadam> (raw)

[-- Attachment #1: Type: text/plain, Size: 12702 bytes --]

tree:   https://github.com/intel/linux-intel-lts.git 5.4/yocto
head:   eeb611e5394c56d45c5cc8f7dc484c9f19e93143
commit: d2a205db2c4ee2728f9be6d45f2361f05205408e [13/1142] crypto: keembay: Add Keem Bay offload ECC Driver
config: i386-randconfig-m021-20201211 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/crypto/keembay/keembay-ocs-ecc-core.c:659 kmb_ocs_ecdh_set_secret() error: we previously assumed 'params.key' could be null (see line 642)
drivers/crypto/keembay/keembay-ocs-ecc-core.c:680 kmb_ocs_ecc_do_one_request() warn: this array is probably non-NULL. 'ctx->private_key'

vim +659 drivers/crypto/keembay/keembay-ocs-ecc-core.c

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  619  static int kmb_ocs_ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  620  				   unsigned int len)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  621  {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  622  	struct ecdh params;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  623  	unsigned int ndigits;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  624  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  625  	int ret = 0;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  626  	struct ocs_ecc_ctx *ctx = kpp_tfm_ctx(tfm);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  627  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  628  	ret = crypto_ecdh_decode_key(buf, len, &params);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  629  	if (ret)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  630  		goto cleanup;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  631  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  632  	ndigits = kmb_ocs_ecdh_supported_curve(params.curve_id);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  633  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  634  	if (!ndigits) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  635  		ret = -EOPNOTSUPP;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  636  		goto cleanup;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  637  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  638  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  639  	ctx->curve_id = params.curve_id;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  640  	ctx->ndigits = ndigits;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  641  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12 @642  	if (!params.key || !params.key_size) {
                                                             ^^^^^^^^^^
"params.key" is NULL.

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  643  		ret = -EINVAL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  644  #ifdef	CONFIG_CRYPTO_DEV_KEEMBAY_OCS_ECDH_GEN_PRIV_KEY_SUPPORT
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  645  		ret = kmb_ecc_gen_privkey(ctx->curve_id, ctx->ndigits,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  646  					  ctx->private_key);

We allocate a new key, but I don't see where it is assigned to
"params.key".

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  647  #endif  /* CONFIG_CRYPTO_DEV_KEEMBAY_OCS_ECDH_GEN_PRIV_KEY_SUPPORT */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  648  		if (ret)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  649  			goto cleanup;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  650  		goto swap_digits;
                                                                ^^^^^^^^^^^^^^^^^

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  651  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  652  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  653  	ret = kmb_ecc_is_key_valid(ctx->curve_id, ctx->ndigits,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  654  				   (const u64 *)params.key, params.key_size);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  655  	if (ret)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  656  		goto cleanup;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  657  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  658  swap_digits:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12 @659  	ecc_swap_digits((u64 *)params.key, ctx->private_key, ctx->ndigits);
                                                                         ^^^^^^^^^^^^^^^^
NULL dereference.

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  660  cleanup:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  661  	memzero_explicit(&params, sizeof(params));
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  662  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  663  	return ret;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  664  }
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  665  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  666  static int kmb_ocs_ecc_do_one_request(struct crypto_engine *engine,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  667  				      void *areq)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  668  {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  669  	size_t copied, nbytes;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  670  	struct ecc_point *pk;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  671  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  672  	u64 *public_key = NULL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  673  	int ret = -ENOMEM;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  674  	size_t public_key_sz = 0;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  675  	struct kpp_request *req = container_of(areq, struct kpp_request,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  676  					       base);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  677  	struct ocs_ecc_ctx *ctx = kmb_ocs_kpp_ctx(req);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  678  	const struct ecc_curve *curve = ecc_get_curve(ctx->curve_id);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  679  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12 @680  	if (!ctx->private_key || !curve ||
                                                             ^^^^^^^^^^^^^^^^
Delete this check.

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  681  	    (ctx->ndigits > KMB_ECC_MAX_DIGITS)) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  682  		ret = -EINVAL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  683  		goto out;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  684  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  685  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  686  	/* No spurious request checked at top level.*/
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  687  	if ((!req->src) && (!req->dst)) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  688  		ret = -EINVAL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  689  		goto out;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  690  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  691  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  692  	/* Store the request flag in ctx. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  693  	ctx->gfp = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ? GFP_KERNEL
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  694  								: GFP_ATOMIC;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  695  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  696  	pk = ecc_alloc_point(ctx->ndigits, ctx->gfp);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  697  	if (!pk) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  698  		ret = -ENOMEM;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  699  		goto out;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  700  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  701  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  702  	/* Store the kpp_request struct in the device context. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  703  	ctx->ecc_dev->req = req;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  704  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  705  	/* In case shared_secret branch not taken. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  706  	ctx->pk = NULL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  707  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  708  	nbytes = data_size_u64_to_u8(ctx->ndigits);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  709  	/* Public part is a point thus it has both coordinates */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  710  	public_key_sz = 2 * nbytes;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  711  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  712  	public_key = kzalloc(public_key_sz, ctx->gfp);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  713  	if (!public_key) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  714  		ret = -ENOMEM;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  715  		goto free_all;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  716  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  717  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  718  	if (req->src) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  719  		/* from here on it's invalid parameters */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  720  		ret = -EINVAL;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  721  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  722  		/* must have exactly two points to be on the curve */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  723  		if (public_key_sz != req->src_len)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  724  			goto free_all;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  725  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  726  		copied = sg_copy_to_buffer(req->src,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  727  					   sg_nents_for_len(req->src,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  728  							    public_key_sz),
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  729  					   public_key, public_key_sz);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  730  		if (copied != public_key_sz)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  731  			goto free_all;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  732  		/* Store pk in the device context. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  733  		ctx->pk = pk;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  734  		ecc_swap_digits(public_key, pk->x, ctx->ndigits);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  735  		ecc_swap_digits(&public_key[ctx->ndigits], pk->y, ctx->ndigits);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  736  		/*
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  737  		 * Check the public key for following
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  738  		 * Check 1: Verify key is not the zero point.
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  739  		 * Check 2: Verify key is in the range [1, p-1].
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  740  		 * Check 3: Verify that y^2 == (x^3 + a·x + b) mod p
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  741  		 */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  742  		ret = kmb_ocs_ecc_is_pubkey_valid_partial(ctx->ecc_dev, curve,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  743  							  pk);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  744  	} else {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  745  		/* Public Key(pk) = priv * G. */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  746  		ret = ecc_point_mult(ctx->ecc_dev, pk, &curve->g,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  747  				     ctx->private_key, curve, ctx->ndigits);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  748  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  749  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  750  	if (ret)
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  751  		goto free_all;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  752  	goto return_success;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  753  
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  754  	/* follow through */
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  755  free_all:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  756  	ecc_free_point(pk);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  757  out:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  758  	crypto_finalize_kpp_request(ctx->ecc_dev->engine,
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  759  				    req, ret);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  760  return_success:
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  761  	if (public_key) {
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  762  		memzero_explicit(public_key, public_key_sz);
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  763  		kzfree(public_key);
                                                                ^^^^^^^^^^^^^^^^^^
I feel like memzero_explicit() and kzfree() are hopefully duplicative.
What is the point of kzfree() if it doesn't have a guaranteed memzero()?

d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  764  	}
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  765  	return 0;
d2a205db2c4ee2 Prabhjot Khurana 2020-06-12  766  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28123 bytes --]

next             reply	other threads:[~2020-12-11  8:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-11  8:13 Dan Carpenter [this message]
2020-12-11  8:13 ` [intel-linux-intel-lts:5.4/yocto 13/1142] drivers/crypto/keembay/keembay-ocs-ecc-core.c:659 kmb_ocs_ecdh_set_secret() error: we previously assumed 'params.key' could be null (see line 642) Dan Carpenter
  -- strict thread matches above, loose matches on Subject: below --
2020-12-11  1:59 kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201211081352.GP2767@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=kbuild@lists.01.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.