linux-kernel - Re: [PATCH v2] usb: serial: Repair FTDI FT232R bricked eeprom

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: <ca3dfd1e-b595-18aa-3442-30363b2e2797@marcansoft.com>
Date:   Thu, 10 Sep 2020 17:10:20 +0900
From:   Hector Martin <hector@...cansoft.com>
To:     James Hilliard <james.hilliard1@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-usb@...r.kernel.org, Johan Hovold <johan@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Russ Dill <Russ.Dill@...il.com>
Subject: Re: [PATCH v2] usb: serial: Repair FTDI FT232R bricked eeprom

On 10/09/2020 15.45, James Hilliard wrote:
>>> +static int ftdi_write_eeprom(struct usb_serial_port *port, u8 addr, u16 data)
>>> +{
>>> +     struct usb_device *udev = port->serial->dev;
>>> +     int rv;
>>> +
>>> +     rv = usb_control_msg(udev,
>>> +                          usb_sndctrlpipe(udev, 0),
>>> +                          FTDI_SIO_WRITE_EEPROM_REQUEST,
>>> +                          FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE,
>>> +                          data, addr,
>>> +                          NULL, 0, WDR_TIMEOUT);
>>> +     if (rv < 0)
>>> +             dev_err(&port->dev, "Unable to write EEPROM: %i\n", rv);
>>
>> You don't check for a "short write"?
>  From my understanding the hardware only accepts 2 byte writes, and
> the non-counterfeits actually only commit writes on odd addresses
> while they buffer writes on even(this difference is what FTDI's windows
> driver exploits). So I guess this should be "if (rv < 2)"?

It's not "data" anyway, the data word gets sent in control message 
headers. Unless I'm mistaken rv == 0 on success, so the code should be 
correct as-is.

>>
>>> +     return rv;
>>> +}
>>> +
>>> +static u16 ftdi_checksum(u16 *data, int n)
>>> +{
>>> +     u16 checksum;
>>> +     int i;
>>> +
>>> +     checksum = 0xaaaa;
>>> +     for (i = 0; i < n - 1; i++) {
>>> +             checksum ^= be16_to_cpu(data[i]);
>>> +             checksum = (checksum << 1) | (checksum >> 15);
>>> +     }
>>
>> What type of function is this, don't we have all of the needed checksum
>> functions in the kernel already?
> Some custom crc16 style checksum I guess, I'm not seeing anything
> in the kernel that's the same, although I might not be looking in the
> right places.

This isn't a CRC, it's some random xor all the words thing with a 
somewhat pointless rotation in the way. I'd be surprised if anything 
elses uses this particular function. Pretty sure other drivers are 
littered with stuff like this too, hardware manufacturers love to 
reinvent checksums.

-- 
Hector Martin (hector@...cansoft.com)
Public Key: https://mrcn.st/pub