linux-kernel - Re: [PATCH] comedi: adl_pci9111: Fix possible division by zero in pci9111_ai_do_cmd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4f46343a-a1f9-4082-8ef2-50cdb3d74f31@mev.co.uk>
Date: Wed, 18 Sep 2024 12:41:00 +0100
From: Ian Abbott <abbotti@....co.uk>
To: Aleksandr Mishin <amishin@...rgos.ru>,
 H Hartley Sweeten <hsweeten@...ionengravers.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH] comedi: adl_pci9111: Fix possible division by zero in
 pci9111_ai_do_cmd_test()

On 18/09/2024 11:43, Aleksandr Mishin wrote:
> Division by zero is possible in pci9111_ai_do_cmd_test() in case of scan
> begin trigger source is TRIG_TIMER and either 'chanlist_len' or
> 'convert_arg' is zero.
> 
> Add zero value check to prevent division by zero.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: f1c51faabc4d ("staging: comedi: adl_pci9111: tidy up (*do_cmdtest) Step 4")
> Signed-off-by: Aleksandr Mishin <amishin@...rgos.ru>
> ---
>   drivers/comedi/drivers/adl_pci9111.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/comedi/drivers/adl_pci9111.c b/drivers/comedi/drivers/adl_pci9111.c
> index 086d93f40cb9..ec1fb570b98c 100644
> --- a/drivers/comedi/drivers/adl_pci9111.c
> +++ b/drivers/comedi/drivers/adl_pci9111.c
> @@ -312,6 +312,8 @@ static int pci9111_ai_do_cmd_test(struct comedi_device *dev,
>   	 */
>   	if (cmd->scan_begin_src == TRIG_TIMER) {
>   		arg = cmd->chanlist_len * cmd->convert_arg;
> +		if (!arg)
> +			return 4;
>   
>   		if (arg < cmd->scan_begin_arg)
>   			arg *= (cmd->scan_begin_arg / arg);

Nice catch!  `cmd->convert_arg` will be non-zero due to earlier checks, 
but `cmd->chanlist_len` could be zero for the `COMEDI_CMDTEST` ioctl. 
(The function is called for the `COMEDI_CMDTEST` and `COMEDI_CMD` 
ioctls, but only `COMEDI_CMD` checks that `chanlist_len` is non-zero 
before calling the function.)

Reviewed-by: Ian Abbott <abbotti@....co.uk>

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