Re: [PATCH v7 03/10] crypto: pkcs7: add tests for pkcs7_get_authattr

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Test failures in `pkcs7_preparse` are silently masked because the function unconditionally returns 0 instead of propagating the error code.
--

commit 49875871a24595817e686cb5ad55b01eebf05536
Author: James Bottomley <James.Bottomley@HansenPartnership.com>

crypto: pkcs7: add tests for pkcs7_get_authattr

Add example code to the test module pkcs7_key_type.c that verifies a message
and then pulls out a known authenticated attribute.

> diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
> index b930d3bbf1af5..e0b1ce0202f6d 100644
> --- a/crypto/asymmetric_keys/pkcs7_key_type.c
> +++ b/crypto/asymmetric_keys/pkcs7_key_type.c

[ ... ]

> @@ -51,16 +52,57 @@ static int pkcs7_view_content(void *ctx, const void *data, size_t len,
>  static int pkcs7_preparse(struct key_preparsed_payload *prep)
>  {

[ ... ]

> +	/*
> +	 * the parsed message has no trusted signer, so nothing should
> +	 * be returned here
> +	 */
> +	ret = pkcs7_get_authattr(pkcs7, OID_messageDigest, &data, &len);
> +	if (ret == 0) {
> +		pr_err("OID returned when no trust in signer\n");
> +		goto out;

If the test fails here, does this need to set ret to a negative error code
like -EINVAL before jumping to the out label? Since ret is 0 here, returning
ret at the end would still signal success.

> +	}
> +	/* add trust and check again */
> +	ret = verify_pkcs7_message_sig(NULL, 0, pkcs7,
> +				       VERIFY_USE_SECONDARY_KEYRING, usage,
> +				       NULL, NULL);
> +	if (ret) {
> +		pr_err("verify_pkcs7_message_sig failed!!\n");
> +		goto out;
> +	}
> +	/* now we should find the OID */
> +	ret = pkcs7_get_authattr(pkcs7, OID_messageDigest, &data, &len);
> +	if (ret) {
> +		pr_err("Failed to get message digest\n");
> +		goto out;
> +	}
> +	pr_info("Correctly Got message hash, size=%zu\n", len);
> +
> + out:
> +	pkcs7_free_message(pkcs7);
> +	return 0;

Should this return ret instead of 0? If any of the earlier assertions fail
and jump to the out label, returning 0 unconditionally discards the error
state and signals a successful key instantiation to the keys subsystem.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507191416.2984054-1-bboscaccy@linux.microsoft.com?part=3