lkml.org 
[lkml]   [2014]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
Looks good Russ, just a minor fix:

- /* 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);
+ /* Attempt to set Product ID */
+ switch (priv->chip_type) {
+ case FT232BM:
+ case FT232RL:
+ case FT232H:
+ if (eeprom_data[2] == 0) {
+ eeprom_data[2] = FTDI_232RL_PID;
+ /* Calculate new checksum to avoid bricking devices */
+ checksum = ftdi_checksum(eeprom_data, eeprom_size);
+
+ /* Verify EEPROM programming behavior/nonbehavior */
+ write_eeprom(port, 2, eeprom_data[2]);
+ write_eeprom(port, 3, eeprom_data[3]);
+ write_eeprom(port, eeprom_size - 2, eeprom_data[eeprom_size - 2]);
+ write_eeprom(port, eeprom_size - 1, checksum);
+ }
+ break;
+ default:
+ break;
+ }

end_verify:

On Thu, Oct 23, 2014 at 10:14 PM, Russ Dill <russ.dill@gmail.com> wrote:
> 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.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


\
 
 \ /
  Last update: 2014-10-24 09:01    [W:0.081 / U:27.476 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site