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

[Ian Abbott <abbotti@mev.co.uk>]

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.
> 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.
> 
> 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.
> 
> Assisted-by: Codex:gpt-5.5-cyber-preview
> Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> ---
>   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);
>   	}

If it is done that way, it is possible for scan_begin_arg to end up 
shorter than scan_end_arg * convert_arg (when both scan_begin_src and 
convert_src are set to TRIG_TIMER).  I prefer to impose an upper limit 
on convert_arg so that the product of scan_end_arg and convert_arg is <= 
UINT_MAX.

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || 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 )=-