Re: [PATCH v3] virt: tdx-guest: Use read_poll_timeout() to shorten get-quote polling interval

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] virt: tdx-guest: successful quote dropped on pending signal
--

commit 1145f13c13d7adf73665778fb8d08dcfab69ccfc
Author: Jun Miao <jun.miao@intel.com>

virt: tdx-guest: Use read_poll_timeout() to shorten get-quote polling interval

This commit reduces the get-quote polling interval from 1 second to 2
milliseconds to improve performance and throughput. It introduces
read_poll_timeout() to handle the polling and timeout logic. The maximum wait
time is also reduced from 30 seconds to 1 second.

> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index d0303e31e8161..bac7cb9403f94 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c

[ ... ]

> @@ -251,18 +252,29 @@ static void *alloc_quote_buf(void)
>   */
>  static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeout)
>  {
> -	int i = 0;
> +	u64 status;
> +	int ret;
>  
>  	/*
> -	 * Quote requests usually take a few seconds to complete, so waking up
> -	 * once per second to recheck the status is fine for this use case.
> +	 * Quote requests usually take a few milliseconds to complete, so waking
> +	 * up once per 2 milliseconds to recheck the status is fine for this use
> +	 * case.
> +	 *
> +	 * Also break out early if a signal is pending so the caller can be
> +	 * interrupted while waiting.
>  	 */
> -	while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) {
> -		if (msleep_interruptible(MSEC_PER_SEC))
> -			return -EINTR;
> -	}
> +	ret = read_poll_timeout(READ_ONCE, status,
> +				status != GET_QUOTE_IN_FLIGHT ||
> +					signal_pending(current),
> +				2 * USEC_PER_MSEC, timeout * USEC_PER_SEC,
> +				false, quote_buf->status);
> +	if (ret)
> +		return ret;
> +
> +	if (signal_pending(current))
> +		return -EINTR;

[Severity: Medium]
Does this code inadvertently discard a successful quote if a signal happens to
be pending at the exact same time the quote completes?

When read_poll_timeout() evaluates its break condition:
status != GET_QUOTE_IN_FLIGHT || signal_pending(current)

It will terminate and return 0 if the quote completes successfully, such as
when status is GET_QUOTE_SUCCESS.

However, if a signal is also concurrently pending, the unconditional check
after the loop will return -EINTR instead.

This causes the caller tdx_report_new_locked() to propagate -EINTR, dropping
the successful quote and potentially causing user-space to retry and overwrite
it.

Could we verify that the quote is actually still in-flight before returning
-EINTR? For example, by checking:
if (quote_buf->status == GET_QUOTE_IN_FLIGHT && signal_pending(current))

> -	return (i == timeout) ? -ETIMEDOUT : 0;
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917102202.750895-1-jun.miao@intel.com?part=1