Re: [v6 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 are silently masked because `pkcs7_preparse` unconditionally returns success on failure paths.
--

commit e5c9359ab8a0636cf9b8d7481caa515ada8b2819
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 to verify a
message and extract a known authenticated attribute.

> diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
> index b930d3bbf1af..e0b1ce0202f6 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)
>  {
>  	enum key_being_used_for usage = pkcs7_usage;

[ ... ]

> +	/*
> +	 * 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 this condition triggers, ret is 0. Would this result in the test
reporting success even if the function were updated to return ret at the
end instead of 0? Should this set a negative error code 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;
>  }

Since all failure paths jump to the out label which unconditionally
returns 0, does this silently mask test failures from the caller?

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