linux-kernel - Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices

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]

Date:	Thu, 23 Oct 2014 07:14:30 -0700
From:	Russ Dill <russ.dill@...il.com>
To:	Hector Martin <hector@...cansoft.com>
Cc:	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices

On Thu, Oct 23, 2014 at 5:44 AM, Hector Martin <hector@...cansoft.com> wrote:
> NAK. This patch neither accomplishes what FTDI intended, nor what the
> author humorously intended.
>
>> +     /* Attempt to set Vendor ID to 0 */
>> +     eeprom_data[1] = 0;
>> +
>> +     /* Calculate new checksum to avoid bricking devices */
>> +     checksum = ftdi_checksum(eeprom_data, eeprom_size);
>> +
>> +     /* Verify EEPROM programming behavior/nonbehavior */
>> +     write_eeprom(port, 1, 0);
>> +     write_eeprom(port, eeprom_size - 1, checksum);
>
> FTDI's verification routine sets the Product ID (at address 2) to 0 and
> a dummy word (at address 0x3e) to a correctly crafted value that makes
> the existing checksum pass. This bricks clone devices (setting PID to
> 0), while original FT232RL devices are not affected as they only commit
> writes when they receive a write command to an odd EEPROM address,
> combining it with the most recently issued write to an even address and
> writing 32 bits at a time.
>
> This patch instead writes the Vendor ID (at address 1) and the real
> checksum (at address 0x3f). As amusing as bricking all devices would be,
> unfortunately, a real FT232RL would just write garbage at addresses 0
> and 0x3e too (as writes are still 32 bits, and no prior even-addressed
> writes have occurred, so the holding register on the chip contains
> garbage). Therefore, the real effect of this patch is to brick clone
> devices (in a different way from the official driver, killing the VID
> instead of the PID), while merely resetting original FT232RL devices to
> defaults, due to the inadvertently corrupted even words now causing a
> checksum mismatch.
>
> Props on the humor, try again with better code next time ;-). I suggest
> the following:
>
> +       write_eeprom(port, 0, eeprom_data[0]);
> +       write_eeprom(port, 1, 0);
> +       write_eeprom(port, eeprom_size - 2, eeprom_data[eeprom_size - 2]);
> +       write_eeprom(port, eeprom_size - 1, checksum);

Damned off by one errors. Yes, it should be the product ID, not the
vendor ID. These write u16's though, writing to wIndex 2 writes to
bytes 4 and 5. the correct code is:

write_eeprom(port, 2, 0);
write_eeprom(port, eeprom_size - 2, checksum);

And yes, the checksum code needs to be modified to create a specially
crafted value that allows the existing checksum to pass.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/