lkml.org 
[lkml]   [2014]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
From
On Thu, Oct 23, 2014 at 5:44 AM, Hector Martin <hector@marcansoft.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.


\
 
 \ /
  Last update: 2014-10-23 16:41    [W:0.055 / U:0.328 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site