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

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Test failures are silently ignored, causing key preparsing to always succeed instead of propagating error codes.
--

commit c10dc39561379c012380a3ebdc109e9849c1a856
Author: James Bottomley <James.Bottomley@HansenPartnership.com>

crypto: pkcs7: add tests for pkcs7_get_authattr

This commit adds example code to the pkcs7_key_type test module. It verifies
a PKCS#7 message and extracts a known authenticated attribute. This helps
validate the pkcs7_get_authattr functionality.

> diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
> --- 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)
>  {
>  	enum key_being_used_for usage = pkcs7_usage;
> +	int ret;
> +	struct pkcs7_message *pkcs7;

[ ... ]

> +	/*
> +	 * 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 OID is incorrectly returned without trust, ret is 0 when jumping to
the out label. Would this still result in a success code even if the function
were updated to return ret instead of 0?

Could ret be set to an error code like -EINVAL before the goto?

> +	/* 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;
>  }

Does this unconditionally return success? 

If a test fails and jumps to the out label, the error code in ret seems to be
discarded. Should this return ret instead?

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