Re: [PATCH] comedi: comedi_test: avoid AI scan timing overflow

[Ian Abbott]

On 09/06/2026 01:14, Samuel Moelius wrote:
> `waveform_ai_cmdtest()` tries to keep timer-driven analog-input scans
> representable by limiting `convert_arg` and by making `scan_begin_arg`
> at least `convert_arg * scan_end_arg`.
>
> The conversion clamp tested `scan_begin_arg == TRIG_TIMER` instead of
> `scan_begin_src == TRIG_TIMER`, so normal timer scans skipped the clamp.

That was fixed in commit 8a3bee801d42 ("comedi: comedi_test: Fix 
limiting of convert_arg in waveform_ai_cmdtest()").

> The later product was computed in `unsigned int`, allowing a large
> conversion period to wrap and produce an accepted command whose true
> scan conversion time exceeds the scan period.

Commit 783ddaebd397 ("staging: comedi: comedi_test: support 
scan_begin_src == TRIG_FOLLOW") imposed an upper limit on `convert_arg` 
to stop the product `convert_arg * scan_end_arg` overflowing an 
`unsigned int`.

> Require timer conversions to be at least one microsecond, apply the
> conversion limit when `scan_begin_src` is `TRIG_TIMER`, keep that limit
> on the same microsecond granularity as the rounded argument, and compute
> the scan-period floor in 64-bit before clamping it back to the ioctl
> argument range.

We could do that but it doesn't really matter for this driver.  It 
already supports `convert_src == TRIG_NOW` with `convert_src == 0` and 
that is pretty much the same as `convert_src == TRIG_TIMER` with 
`convert_src == 0`.  The current implementation just allows 
`convert_arg` to be arbitrarily small (including 0) when `convert_src == 
TRIG_TIMER` except when `scan_begin_src == TRIG_FOLLOW` because that 
would also make the overall scan period arbitrarily small, which we do 
not want.

> Assisted-by: Codex:gpt-5.5-cyber-preview
> Signed-off-by: Samuel Moelius <sam.moelius@xxxxxxxxxxxxxxx>
> ---
>   drivers/comedi/drivers/comedi_test.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/comedi/drivers/comedi_test.c b/drivers/comedi/drivers/comedi_test.c
> index 1f430ffc7bd9..04b982f69751 100644
> --- a/drivers/comedi/drivers/comedi_test.c
> +++ b/drivers/comedi/drivers/comedi_test.c
> @@ -256,7 +256,8 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
>   			       struct comedi_cmd *cmd)
>   {
>   	int err = 0;
> -	unsigned int arg, limit;
> +	unsigned int arg, limit, min_scan_arg;
> +	u64 min_scan_time;
>   
>   	/* Step 1 : check if triggers are trivially valid */
>   
> @@ -292,10 +293,8 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
>   	if (cmd->convert_src == TRIG_NOW) {
>   		err |= comedi_check_trigger_arg_is(&cmd->convert_arg, 0);
>   	} else {	/* cmd->convert_src == TRIG_TIMER */
> -		if (cmd->scan_begin_src == TRIG_FOLLOW) {
> -			err |= comedi_check_trigger_arg_min(&cmd->convert_arg,
> -							    NSEC_PER_USEC);
> -		}
> +		err |= comedi_check_trigger_arg_min(&cmd->convert_arg,
> +						    NSEC_PER_USEC);
>   	}
>   
>   	if (cmd->scan_begin_src == TRIG_FOLLOW) {
> @@ -342,7 +341,12 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
>   		arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
>   		if (cmd->convert_src == TRIG_TIMER) {
>   			/* but ensure scan_begin_arg is large enough */
> -			arg = max(arg, cmd->convert_arg * cmd->scan_end_arg);
> +			min_scan_time = (u64)cmd->convert_arg * cmd->scan_end_arg;
> +			if (min_scan_time > UINT_MAX)
> +				min_scan_arg = UINT_MAX;
> +			else
> +				min_scan_arg = min_scan_time;
> +			arg = max(arg, min_scan_arg);
>   		}
>   		err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, arg);
>   	}


--
-=( Ian Abbott <abbotti@xxxxxxxxx> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-