[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)
7:13 p.m.
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(a)intel.com>
Reported-by: Dan Carpenter <dan.carpenter(a)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,
¶ms);
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(¶ms,
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@lists.01.org
648
days inactive
648
days old
0 comments
1 participants
participants (1)
-
Dan Carpenter