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

sashiko-bot
2026-04-16T20:17:47Z

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