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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111102113104.3a641f7a@bob.linux.org.uk>
Date:	Wed, 2 Nov 2011 11:31:04 +0000
From:	Alan Cox <alan@...ux.intel.com>
To:	Andrew Worsley <amworsley@...il.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

> >> +       /* compare old_termios and tty->termios */
> >> +    if (old_termios->c_cflag == termios->c_cflag)
> >> +        goto no_c_cflag_changes;
> >
> > You can't do it this way because the speed data is not entirely
> > within c_cflag. Check c_ispeed and c_ospeed match and for the
> > parity if you want to skip that check if the parity bits change
> > specifically.
> >
> This is getting into magic flags I don't understand. There are so few
> bits in c_cflag not related to speed and data/parity I am hesitant  to
> write a complex check I might well get wrong. But flow control appears
> to be switched off / on frequently during data flow,

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.

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

That is safe enough

> +
>         /* 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.

Alan

--
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/