Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200909193419.2006744-1-james.hilliard1@gmail.com> <1599706954.10822.3.camel@suse.de>
In-Reply-To: <1599706954.10822.3.camel@suse.de>
From:   James Hilliard <james.hilliard1@gmail.com>
Date:   Wed, 9 Sep 2020 21:40:59 -0600
Message-ID: <CADvTj4oznyY=sfbmtmdcYMgkYriV_Ujk=+0Fz--XumNa8ZvD=A@mail.gmail.com>
Subject: Re: [PATCH v2] usb: serial: Repair FTDI FT232R bricked eeprom
To:     Oliver Neukum <oneukum@suse.de>
Cc:     linux-usb@vger.kernel.org, Johan Hovold <johan@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Russ Dill <Russ.Dill@gmail.com>,
        Hector Martin <hector@marcansoft.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Sep 9, 2020 at 9:02 PM Oliver Neukum <oneukum@suse.de> wrote:
>
> Am Mittwoch, den 09.09.2020, 13:34 -0600 schrieb James Hilliard:
> > This patch detects and reverses the effects of the malicious FTDI
> > Windows driver version 2.12.00(FTDIgate).
>
> Hi,
>
> this raises questions.
> Should we do this unconditionally without asking?
Well I think since we can reliably detect devices that have been
bricked by the malicious windows driver it's fine. I was careful to
ensure that this will bail out and not try to change anything unless all
conditions match this specific brick attack.
> Does this belong into kernel space?
This seemed to be by far the simplest option for embedded systems
that need to automatically detect and repair the bricked eeproms.

People seem to have plugged a bunch of counterfeit FTDI Arduino's
that normally attach to an embedded Linux host into windows for
some reason for a kiosk platform of mine which messed up the
userspace detection/mappings. This seemed like the best way to
avoid this being a support issue requiring manual unbricking
prochedures.
>
> > +static int ftdi_repair_brick(struct usb_serial_port *port)
> > +{
> > +     struct ftdi_private *priv = usb_get_serial_port_data(port);
> > +     int orig_latency;
> > +     int rv;
> > +     u16 *eeprom_data;
> > +     u16 checksum;
> > +     int eeprom_size;
> > +     int result;
> > +
> > +     switch (priv->chip_type) {
> > +     case FT232RL:
> > +             eeprom_size = 0x40;
> > +             break;
> > +     default:
> > +             /* Unsupported for brick repair */
> > +             return 0;
> > +     }
> > +
> > +     /* Latency timer needs to be 0x77 to unlock EEPROM programming */
> > +     if (priv->latency != 0x77) {
> > +             orig_latency = priv->latency;
> > +             priv->latency = 0x77;
> > +             rv = write_latency_timer(port);
> > +             priv->latency = orig_latency;
> > +             if (rv < 0)
> > +                     return -EIO;
> > +     }
>
> Do you really want to change this without returning to the original?
> @@ -2255,6 +2364,12 @@ 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;
> +       vendor_id = le16_to_cpu(port->serial->dev->descriptor.idVendor);
> +       product_id = le16_to_cpu(port->serial->dev->descriptor.idProduct);
> +       if (vendor_id == FTDI_VID &&
> +               product_id == FTDI_BRICK_PID &&
> +               priv->chip_type == FT232RL)
> +               ftdi_repair_brick(port);
>         write_latency_timer(port);
It should get restored here.
>         create_sysfs_attrs(port);
>
>
>         Regards
>                 Oliver
>