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

From: Hector Martin
Date: Thu Sep 10 2020 - 04:10:35 EST

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),
+ data, addr,
+ 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@xxxxxxxxxxxxxx)
Public Key: