linux-kernel - Re: [PATCH] Fix Corruption issue in USB ftdi driver drivers/usb/serial/ftdi

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]

Message-ID: <CA+Y=x3=iPBW4UcWbsD2VX3nfbxwMBCwAYS4176iseZ-Niy+oKw@mail.gmail.com>
Date:	Thu, 3 Nov 2011 16:06:31 +1100
From:	Andrew Worsley <amworsley@...il.com>
To:	Alan Cox <alan@...ux.intel.com>
Cc:	Greg Kroah-Hartman <gregkh@...e.de>,
	Uwe Bonnes <bon@...ktron.ikp.physik.tu-darmstadt.de>,
	Johan Hovold <jhovold@...il.com>,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix Corruption issue in USB ftdi driver drivers/usb/serial/ftdi_sio.c

Avoid unnecessary Control URBs that reset the data/parity or baud rate
to the currently set settings which can cause the FTDI chip to glitch
it's serial output and cause a corruption of a character it is
currently outputting.

Signed-off-by: amworsley@...il.com

---

....
> It shouldn't be unless the apps you are running are doing odd things.
> I'd not expect parity to keep changing certainly. What may be occuring
> is that some of these devices only handle RTS/CTS flow and force the
> flag on, indicating it back to the app. If the app ignores that then it
> may end up trying to clear it several times.

Okay - Thanks I will try stracing the process and see if it is issuing
all the requests. I thought
it might be the line discipline trying to flow control the input. I've
lost my box for the moment so that
will be tomorrow perhaps.

....

>> +
>>         /* NOTE These routines can get interrupted by
>>            ftdi_sio_read_bulk_callback  - need to examine what this
>> means - don't see any problems yet */
>>
>> +    if ((old_termios->c_cflag & (CSIZE|PARODD|CSTOPB|PARODD)) ==
>> +        (termios->c_cflag & (CSIZE|PARODD|CSTOPB|PARODD)))
>
> I think you need
>
> CSIZE (for CS7/8 switch)
> PARODD (parity odd/even)
> CMSPAR (parity mark/space v odd/even)
> CSTOPB (stop bits)
>
> while you have PARODD twice.
>
> Otherwise this looks correct.


  Woops - yep that was wrong. I attach a new patch - with CMSPAR and
PARENB which I assume is also parity related.

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index bda9e5b..89dbf8a 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -2105,13 +2105,19 @@ static void ftdi_set_termios(struct tty_struct *tty,
        cflag = termios->c_cflag;

        /* compare old_termios and tty->termios */
-    if (old_termios->c_cflag == termios->c_cflag)
+    if (old_termios->c_cflag == termios->c_cflag
+      && old_termios->c_ispeed == termios->c_ispeed
+      && old_termios->c_ospeed == termios->c_ospeed
+      )
         goto no_c_cflag_changes;

        /* NOTE These routines can get interrupted by
           ftdi_sio_read_bulk_callback  - need to examine what this means -
           don't see any problems yet */

+    if ((old_termios->c_cflag & (CSIZE|PARODD|PARENB|CMSPAR|CSTOPB)) ==
+        (termios->c_cflag & (CSIZE|PARODD|PARENB|CMSPAR|CSTOPB)))
+        goto no_data_parity_stop_changes;
        /* Set number of data bits, parity, stop bits */

        urb_value = 0;
@@ -2151,6 +2157,7 @@ static void ftdi_set_termios(struct tty_struct *tty,
                        "databits/stopbits/parity\n", __func__);
        }

+no_data_parity_stop_changes:
        /* Now do the baudrate */
        if ((cflag & CBAUD) == B0) {
                /* Disable flow control */
--
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/