linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: serial: Perform verification for FTDI FT232R devices
@ 2014-10-23  8:52 russ.dill
  2014-10-23  9:40 ` Greg KH
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: russ.dill @ 2014-10-23  8:52 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-usb, greg, Russ Dill

From: Russ Dill <Russ.Dill@gmail.com>

This patch provides the FTDI genuine product verification steps
as contained within the new 2.12.00 official release. It ensures
that counterfeiters don't exploit engineering investment made
by FTDI. Counterfeit ICs are destroying innovation in the
industry.

FTDI recommends that to guarantee genuine FTDI products
please purchase either from FTDI directly or an authorised
distributor.

This is definitely not targeting end users - if you're unsure if
ICs are genuine then please don't use the drivers.

Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
---
 drivers/usb/serial/ftdi_sio.c | 111 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/serial/ftdi_sio.h |  41 ++++++++++++++++
 2 files changed, 151 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index dc72b924c399..ef8b0dd632d3 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1377,6 +1377,48 @@ static int read_latency_timer(struct usb_serial_port *port)
 	return rv;
 }
 
+static int 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);
+	return rv;
+}
+
+static int read_eeprom(struct usb_serial_port *port, u8 addr, u16 *data)
+{
+	struct usb_device *udev = port->serial->dev;
+	u16 *buf;
+	int rv;
+
+	buf = kmalloc(2, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	rv = usb_control_msg(udev,
+			     usb_rcvctrlpipe(udev, 0),
+			     FTDI_SIO_GET_LATENCY_TIMER_REQUEST,
+			     FTDI_SIO_GET_LATENCY_TIMER_REQUEST_TYPE,
+			     0, addr,
+			     buf, 2, WDR_TIMEOUT);
+	if (rv < 0)
+		dev_err(&port->dev, "Unable to read from EEPROM: %i\n", rv);
+	else
+		*data = *buf;
+
+	kfree(buf);
+
+	return rv;
+}
+
 static int get_serial_info(struct usb_serial_port *port,
 				struct serial_struct __user *retinfo)
 {
@@ -1723,12 +1765,78 @@ static int ftdi_sio_probe(struct usb_serial *serial,
 	return 0;
 }
 
+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);
+	}
+
+	return cpu_to_be16(checksum);
+}
+
+static void ftdi_verify(struct usb_serial_port *port)
+{
+	struct ftdi_private *priv = usb_get_serial_port_data(port);
+	u16 *eeprom_data;
+	u16 checksum;
+	int eeprom_size;
+	int i;
+
+	switch (priv->chip_type) {
+	case FT232RL:
+		eeprom_size = 0x40;
+		break;
+	default:
+		/* Unsupported for verification */
+		return;
+	}
+
+	/* Latency timer needs to be 0x77 to unlock EEPROM programming */
+	if (priv->latency != 0x77) {
+		int orig_latency = priv->latency;
+		priv->latency = 0x77;
+		write_latency_timer(port);
+		priv->latency = orig_latency;
+	}
+
+	eeprom_data = kzalloc(eeprom_size * 2, GFP_KERNEL);
+	if (!eeprom_data)
+		return;
+
+	/* Read in EEPROM */
+	for (i = 0; i < eeprom_size; i++)
+		if (read_eeprom(port, i, eeprom_data + i) < 0)
+			goto end_verify;
+
+	/* Verify EEPROM is valid */
+	checksum = ftdi_checksum(eeprom_data, eeprom_size);
+	if (checksum != eeprom_data[eeprom_size - 1])
+		goto end_verify;
+
+	/* 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);
+
+end_verify:
+	kfree(eeprom_data);
+}
+
 static int ftdi_sio_port_probe(struct usb_serial_port *port)
 {
 	struct ftdi_private *priv;
 	struct ftdi_sio_quirk *quirk = usb_get_serial_data(port->serial);
 
-
 	priv = kzalloc(sizeof(struct ftdi_private), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -1746,6 +1854,7 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
 	ftdi_set_max_packet_size(port);
 	if (read_latency_timer(port) < 0)
 		priv->latency = 16;
+	ftdi_verify(port);
 	write_latency_timer(port);
 	create_sysfs_attrs(port);
 	return 0;
diff --git a/drivers/usb/serial/ftdi_sio.h b/drivers/usb/serial/ftdi_sio.h
index ed58c6fa8dbe..9ed6b7645cae 100644
--- a/drivers/usb/serial/ftdi_sio.h
+++ b/drivers/usb/serial/ftdi_sio.h
@@ -35,6 +35,9 @@
 #define FTDI_SIO_SET_ERROR_CHAR		7 /* Set the error character */
 #define FTDI_SIO_SET_LATENCY_TIMER	9 /* Set the latency timer */
 #define FTDI_SIO_GET_LATENCY_TIMER	10 /* Get the latency timer */
+#define FTDI_SIO_READ_EEPROM		0x90 /* Read 2 bytes from EEPROM */
+#define FTDI_SIO_WRITE_EEPROM		0x91 /* Write 2 bytes to EEPROM */
+
 
 /* Interface indices for FT2232, FT2232H and FT4232H devices */
 #define INTERFACE_A		1
@@ -345,6 +348,44 @@ enum ftdi_sio_baudrate {
  */
 
 /*
+ * FTDI_SIO_READ_EEPROM
+ *
+ * Read 2 bytes of EEPROM data.
+ */
+#define  FTDI_SIO_READ_EEPROM_REQUEST FTDI_SIO_READ_EEPROM
+#define  FTDI_SIO_READ_EEPROM_REQUEST_TYPE 0xC0
+
+/*
+ *  BmRequestType:   1100 0000b
+ *  bRequest:        FTDI_SIO_READ_EEPROM
+ *  wValue:          0
+ *  wIndex:          EEPROM address / 2
+ *  wLength:         0
+ *  Data:            2 bytes of data from EEPROM
+ */
+
+/*
+ * FTDI_SIO_WRITE_EEPROM
+ *
+ * Write 2 bytes of data to EEPROM.
+ */
+#define  FTDI_SIO_WRITE_EEPROM_REQUEST FTDI_SIO_WRITE_EEPROM
+#define  FTDI_SIO_WRITE_EEPROM_REQUEST_TYPE 0x40
+
+/*
+ *  BmRequestType:   0100 0000b
+ *  bRequest:        FTDI_SIO_WRITE_EEPROM
+ *  wValue:          2 bytes for EEPROM
+ *  wIndex:          EEPROM address / 2
+ *  wLength:         0
+ *  Data:            None
+ *
+ * wValue:
+ *   B0..15  EEPROM data
+ *
+ */
+
+/*
  * FTDI_SIO_SET_EVENT_CHAR
  *
  * Set the special event character for the specified communications port.
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23  8:52 [PATCH] usb: serial: Perform verification for FTDI FT232R devices russ.dill
@ 2014-10-23  9:40 ` Greg KH
  2014-10-23  9:53   ` Frans Klaver
  2014-10-23 11:18   ` One Thousand Gnomes
  2014-10-27 10:58 ` Oliver Neukum
  2014-10-27 11:09 ` Oliver Neukum
  2 siblings, 2 replies; 12+ messages in thread
From: Greg KH @ 2014-10-23  9:40 UTC (permalink / raw)
  To: russ.dill; +Cc: linux-kernel, linux-usb

On Thu, Oct 23, 2014 at 01:52:24AM -0700, russ.dill@gmail.com wrote:
> From: Russ Dill <Russ.Dill@gmail.com>
> 
> This patch provides the FTDI genuine product verification steps
> as contained within the new 2.12.00 official release. It ensures
> that counterfeiters don't exploit engineering investment made
> by FTDI. Counterfeit ICs are destroying innovation in the
> industry.
> 
> FTDI recommends that to guarantee genuine FTDI products
> please purchase either from FTDI directly or an authorised
> distributor.
> 
> This is definitely not targeting end users - if you're unsure if
> ICs are genuine then please don't use the drivers.
> 
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> ---
>  drivers/usb/serial/ftdi_sio.c | 111 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/usb/serial/ftdi_sio.h |  41 ++++++++++++++++
>  2 files changed, 151 insertions(+), 1 deletion(-)

Funny patch, you should have saved it for April 1, otherwise people
might have actually taken this seriously :)

Patches as performance art, now I've seen everything...

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23  9:40 ` Greg KH
@ 2014-10-23  9:53   ` Frans Klaver
  2014-10-23 11:18   ` One Thousand Gnomes
  1 sibling, 0 replies; 12+ messages in thread
From: Frans Klaver @ 2014-10-23  9:53 UTC (permalink / raw)
  To: Greg KH; +Cc: russ.dill, linux-kernel, linux-usb

On Thu, Oct 23, 2014 at 11:40 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Oct 23, 2014 at 01:52:24AM -0700, russ.dill@gmail.com wrote:
>> From: Russ Dill <Russ.Dill@gmail.com>
>>
>> This patch provides the FTDI genuine product verification steps
>> as contained within the new 2.12.00 official release. It ensures
>> that counterfeiters don't exploit engineering investment made
>> by FTDI. Counterfeit ICs are destroying innovation in the
>> industry.
>>
>> FTDI recommends that to guarantee genuine FTDI products
>> please purchase either from FTDI directly or an authorised
>> distributor.
>>
>> This is definitely not targeting end users - if you're unsure if
>> ICs are genuine then please don't use the drivers.
>>
>> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
>> ---
>>  drivers/usb/serial/ftdi_sio.c | 111 +++++++++++++++++++++++++++++++++++++++++-
>>  drivers/usb/serial/ftdi_sio.h |  41 ++++++++++++++++
>>  2 files changed, 151 insertions(+), 1 deletion(-)
>
> Funny patch, you should have saved it for April 1, otherwise people
> might have actually taken this seriously :)
>
> Patches as performance art, now I've seen everything...

This just made my day :)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23  9:40 ` Greg KH
  2014-10-23  9:53   ` Frans Klaver
@ 2014-10-23 11:18   ` One Thousand Gnomes
  2014-10-23 11:19     ` Johan Hovold
  2014-10-23 12:55     ` Mark Brown
  1 sibling, 2 replies; 12+ messages in thread
From: One Thousand Gnomes @ 2014-10-23 11:18 UTC (permalink / raw)
  To: Greg KH; +Cc: russ.dill, linux-kernel, linux-usb

> >  drivers/usb/serial/ftdi_sio.c | 111 +++++++++++++++++++++++++++++++++++++++++-
> >  drivers/usb/serial/ftdi_sio.h |  41 ++++++++++++++++
> >  2 files changed, 151 insertions(+), 1 deletion(-)
> 
> Funny patch, you should have saved it for April 1, otherwise people
> might have actually taken this seriously :)
> 
> Patches as performance art, now I've seen everything...

Chuckle. Sillyness aside a pure detection version of that patch might be
useful so it can warn users "Running Windows may damage your adapter" 8)

Is the 0x0000, 0x0401 they end up with consistent - can we add that to the
default table so end users can at least make use of devices that have
been attacked by malware ?

Alan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23 11:18   ` One Thousand Gnomes
@ 2014-10-23 11:19     ` Johan Hovold
  2014-10-23 12:55     ` Mark Brown
  1 sibling, 0 replies; 12+ messages in thread
From: Johan Hovold @ 2014-10-23 11:19 UTC (permalink / raw)
  To: One Thousand Gnomes; +Cc: Greg KH, russ.dill, linux-kernel, linux-usb

On Thu, Oct 23, 2014 at 12:18:15PM +0100, One Thousand Gnomes wrote:
> > >  drivers/usb/serial/ftdi_sio.c | 111 +++++++++++++++++++++++++++++++++++++++++-
> > >  drivers/usb/serial/ftdi_sio.h |  41 ++++++++++++++++
> > >  2 files changed, 151 insertions(+), 1 deletion(-)
> > 
> > Funny patch, you should have saved it for April 1, otherwise people
> > might have actually taken this seriously :)
> > 
> > Patches as performance art, now I've seen everything...
> 
> Chuckle. Sillyness aside a pure detection version of that patch might be
> useful so it can warn users "Running Windows may damage your adapter" 8)
> 
> Is the 0x0000, 0x0401 they end up with consistent - can we add that to the
> default table so end users can at least make use of devices that have
> been attacked by malware ?

Way ahead of you. ;)

	http://marc.info/?l=linux-usb&m=141405129201389&w=2

Johan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23 11:18   ` One Thousand Gnomes
  2014-10-23 11:19     ` Johan Hovold
@ 2014-10-23 12:55     ` Mark Brown
  1 sibling, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-10-23 12:55 UTC (permalink / raw)
  To: One Thousand Gnomes; +Cc: Greg KH, russ.dill, linux-kernel, linux-usb

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Thu, Oct 23, 2014 at 12:18:15PM +0100, One Thousand Gnomes wrote:
> > >  drivers/usb/serial/ftdi_sio.c | 111 +++++++++++++++++++++++++++++++++++++++++-
> > >  drivers/usb/serial/ftdi_sio.h |  41 ++++++++++++++++
> > >  2 files changed, 151 insertions(+), 1 deletion(-)

> > Funny patch, you should have saved it for April 1, otherwise people
> > might have actually taken this seriously :)

> > Patches as performance art, now I've seen everything...

> Chuckle. Sillyness aside a pure detection version of that patch might be
> useful so it can warn users "Running Windows may damage your adapter" 8)

The EEPROM read function might also be interesting too, especially with
a userspace interface for reading it back.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23  8:52 [PATCH] usb: serial: Perform verification for FTDI FT232R devices russ.dill
  2014-10-23  9:40 ` Greg KH
@ 2014-10-27 10:58 ` Oliver Neukum
  2014-10-27 11:09 ` Oliver Neukum
  2 siblings, 0 replies; 12+ messages in thread
From: Oliver Neukum @ 2014-10-27 10:58 UTC (permalink / raw)
  To: russ.dill; +Cc: linux-kernel, linux-usb, greg

On Thu, 2014-10-23 at 01:52 -0700, russ.dill@gmail.com wrote:
> From: Russ Dill <Russ.Dill@gmail.com>
> 
> This patch provides the FTDI genuine product verification steps
> as contained within the new 2.12.00 official release. It ensures
> that counterfeiters don't exploit engineering investment made
> by FTDI. Counterfeit ICs are destroying innovation in the
> industry.
> 
> FTDI recommends that to guarantee genuine FTDI products
> please purchase either from FTDI directly or an authorised
> distributor.
> 
> This is definitely not targeting end users - if you're unsure if
> ICs are genuine then please don't use the drivers.

So you don't want this merged?
If you want to have it merged please explain the benefit.

> +static void ftdi_verify(struct usb_serial_port *port)
> +{
> +	struct ftdi_private *priv = usb_get_serial_port_data(port);
> +	u16 *eeprom_data;
> +	u16 checksum;
> +	int eeprom_size;
> +	int i;
> +
> +	switch (priv->chip_type) {
> +	case FT232RL:
> +		eeprom_size = 0x40;
> +		break;
> +	default:
> +		/* Unsupported for verification */
> +		return;
> +	}
> +
> +	/* Latency timer needs to be 0x77 to unlock EEPROM programming */
> +	if (priv->latency != 0x77) {
> +		int orig_latency = priv->latency;
> +		priv->latency = 0x77;
> +		write_latency_timer(port);
> +		priv->latency = orig_latency;
> +	}
> +
> +	eeprom_data = kzalloc(eeprom_size * 2, GFP_KERNEL);
> +	if (!eeprom_data)
> +		return;
> +
> +	/* Read in EEPROM */
> +	for (i = 0; i < eeprom_size; i++)
> +		if (read_eeprom(port, i, eeprom_data + i) < 0)
> +			goto end_verify;
> +
> +	/* Verify EEPROM is valid */
> +	checksum = ftdi_checksum(eeprom_data, eeprom_size);
> +	if (checksum != eeprom_data[eeprom_size - 1])
> +		goto end_verify;
> +
> +	/* 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);

In case of disconnect here, what have we just done to the device?

> +	write_eeprom(port, eeprom_size - 1, checksum);
> +
> +end_verify:
> +	kfree(eeprom_data);
> +}
> +

	Oliver



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23  8:52 [PATCH] usb: serial: Perform verification for FTDI FT232R devices russ.dill
  2014-10-23  9:40 ` Greg KH
  2014-10-27 10:58 ` Oliver Neukum
@ 2014-10-27 11:09 ` Oliver Neukum
  2 siblings, 0 replies; 12+ messages in thread
From: Oliver Neukum @ 2014-10-27 11:09 UTC (permalink / raw)
  To: russ.dill; +Cc: linux-kernel, linux-usb, greg

On Thu, 2014-10-23 at 01:52 -0700, russ.dill@gmail.com wrote:
> From: Russ Dill <Russ.Dill@gmail.com>
> 
> This patch provides the FTDI genuine product verification steps
> as contained within the new 2.12.00 official release. It ensures
> that counterfeiters don't exploit engineering investment made
> by FTDI. Counterfeit ICs are destroying innovation in the
> industry.

Sorry about not getting the joke. I should read news before
going through a backlog.

	Regards
		Oliver



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23 14:14 ` Russ Dill
  2014-10-23 17:05   ` Hector Martin
@ 2014-10-24  6:22   ` Perry Hung
  1 sibling, 0 replies; 12+ messages in thread
From: Perry Hung @ 2014-10-24  6:22 UTC (permalink / raw)
  To: Russ Dill; +Cc: Hector Martin, LKML

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/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23 14:14 ` Russ Dill
@ 2014-10-23 17:05   ` Hector Martin
  2014-10-24  6:22   ` Perry Hung
  1 sibling, 0 replies; 12+ messages in thread
From: Hector Martin @ 2014-10-23 17:05 UTC (permalink / raw)
  To: Russ Dill; +Cc: LKML

On 23/10/14 23:14, Russ Dill wrote:
> On Thu, Oct 23, 2014 at 5:44 AM, Hector Martin <hector@marcansoft.com> wrote:
>> +       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.

Oh, I thought/assumed that the switch to VID and the real checksum was a
deliberate attempt at perversing the original code into something that
bricks even real FTDIs :-) (In which case all you'd need to add is the
two extra dummy writes, to load the proper data into the buffer register
on a real FT232RL to prevent it from corrupting the adjacent EEPROM words).

-- 
Hector Martin (hector@marcansoft.com)
Public Key: http://www.marcansoft.com/marcan.asc


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
  2014-10-23 12:44 Hector Martin
@ 2014-10-23 14:14 ` Russ Dill
  2014-10-23 17:05   ` Hector Martin
  2014-10-24  6:22   ` Perry Hung
  0 siblings, 2 replies; 12+ messages in thread
From: Russ Dill @ 2014-10-23 14:14 UTC (permalink / raw)
  To: Hector Martin; +Cc: LKML

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] usb: serial: Perform verification for FTDI FT232R devices
@ 2014-10-23 12:44 Hector Martin
  2014-10-23 14:14 ` Russ Dill
  0 siblings, 1 reply; 12+ messages in thread
From: Hector Martin @ 2014-10-23 12:44 UTC (permalink / raw)
  To: LKML; +Cc: russ.dill

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);

This will correctly set the Vendor ID to zero on all devices, counterfeit
or not.

-- 
Hector Martin (hector@marcansoft.com)
Public Key: http://www.marcansoft.com/marcan.asc


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-10-27 11:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-23  8:52 [PATCH] usb: serial: Perform verification for FTDI FT232R devices russ.dill
2014-10-23  9:40 ` Greg KH
2014-10-23  9:53   ` Frans Klaver
2014-10-23 11:18   ` One Thousand Gnomes
2014-10-23 11:19     ` Johan Hovold
2014-10-23 12:55     ` Mark Brown
2014-10-27 10:58 ` Oliver Neukum
2014-10-27 11:09 ` Oliver Neukum
2014-10-23 12:44 Hector Martin
2014-10-23 14:14 ` Russ Dill
2014-10-23 17:05   ` Hector Martin
2014-10-24  6:22   ` Perry Hung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).