Re: [PATCH 05/15] device core: Add "device evidence" 'validate' command

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Holding the global `device_evidence_lock` across the provider's `evidence_read_begin()` callback blocks system-wide operations and causes global lock contention.
--

commit e225a84deccb758c0117766e62b3f41123a9e5e3
Author: Dan Williams <djbw@kernel.org>

device core: Add "device evidence" 'validate' command

When DMA is to be enabled for a PCI TDISP device, the CC VM passes a digest
of the device evidence used to make the trust decision. This commit adds a
validate command to require the CC VM to ack and not invalidate the evidence
before the next DMA enable event.

> diff --git a/drivers/base/evidence.c b/drivers/base/evidence.c
> --- a/drivers/base/evidence.c
> +++ b/drivers/base/evidence.c

[ ... ]

> @@ -446,6 +446,48 @@ int device_evidence_nl_read_dumpit(struct sk_buff *skb,
>  	return device_evidence_read(skb, cb);
>  }
>  
> +int device_evidence_nl_validate_doit(struct sk_buff *skb,
> +				     struct genl_info *info)
> +{
> +	const struct device_evidence_ops *ops;
> +	struct device_evidence *evidence;
> +	struct device_evidence_ctx _ctx;
> +	u32 generation;
> +	int rc;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, DEVICE_EVIDENCE_A_OBJECT_GENERATION)) {
> +		NL_SET_ERR_MSG(info->extack, "missing evidence generation");
> +		return -EINVAL;
> +	}
> +	generation = nla_get_u32(info->attrs[DEVICE_EVIDENCE_A_OBJECT_GENERATION]);
> +
> +	ACQUIRE(mutex_intr, lock)(&device_evidence_lock);
> +	if ((rc = ACQUIRE_ERR(mutex_intr, &lock)))
> +		return rc;
> +
> +	struct device_evidence_ctx *ctx __free(put_ctx) =
> +		device_evidence_ctx_setup(info, &_ctx);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
> +	ops = ctx->subsys->ops;
> +	evidence = ops->evidence_read_begin(ctx->dev);

[Severity: Medium]
Will holding device_evidence_lock across the evidence_read_begin() callback
cause global lock contention and system-wide stalls?

If a slow hardware refresh operation starts on one device and acquires its
per-device evidence lock, and a concurrent validate command on that same
device blocks in evidence_read_begin() while holding the global
device_evidence_lock:

Device A (refresh):
  acquires evidence->lock for write
  (slow hardware communication)

Device A (validate):
  device_evidence_nl_validate_doit()
    acquires device_evidence_lock
    evidence_read_begin()
      blocks on evidence->lock

It seems this would cause any concurrent read or validate commands on unrelated
devices (like Device B) to block on the global device_evidence_lock until
Device A's hardware operation completes.

Could the global lock scope be reduced here to prevent this stall?

> +	if (IS_ERR(evidence)) {
> +		NL_SET_ERR_MSG(info->extack, "failed to acquire evidence context");
> +		return PTR_ERR(evidence);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705220819.2472765-1-djbw@kernel.org?part=5