libftdi Archives

Subject: Re: [PATCH] Fixed first device failure in open_desc function

From: Thomas Jarosch <thomas.jarosch@xxxxxxxxxxxxx>
To: libftdi@xxxxxxxxxxxxxxxxxxxxxxx
Cc: amurray@xxxxxxxxxxxxxxxxxxxx, Harrison Marcks <hmarcks@xxxxxxxxxxxxxxxxxxxx>
Date: Tue, 30 Aug 2022 17:10:06 +0200
Hi Harrison,

thanks for the patch!

I do have a few questions though, I'll put them below
next to the relevant section of the code:

You wrote on Thu, Jul 07, 2022 at 02:30:47PM +0100:
>          if (desc.idVendor == vendor && desc.idProduct == product)
>          {
> +            open_counter++;
>              if (libusb_open(dev, &ftdi->usb_dev) < 0)
> -                ftdi_error_return_free_device_list(-4, "usb_open() failed", 
> devs);
> +            {
> +                open_fail_counter++;
> +                error_code = -4;
> +                continue;
> +            }

why does the open_counter get increased before
and after libusb_open()?


> +        case -4:
> +        {
> +            // Failure to open any devices. We made at least one match and 
> it failed to open.
> +            char buf [256];
> +            sprintf(buf, "usb_open() ran %d times and failed %d times.", 
> open_counter, open_fail_counter);
> +            ftdi_error_return_free_device_list(-4, buf, devs);
> +        }
> +        break;

do we really need the "open_counter" besides the diagnostic here?

I think this code introduces a use-after-free: The buf[256] is
on the stack and that will get reused as soon as the function
returns here. Ideally we can just get rid of the diagnostic?

Cheers,
Thomas

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to libftdi+unsubscribe@xxxxxxxxxxxxxxxxxxxxxxx   

Current Thread