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: <47B40918.20206@davidnewall.com>
Date:	Thu, 14 Feb 2008 19:55:44 +1030
From:	David Newall <davidn@...idnewall.com>
To:	Greg KH <greg@...ah.com>
CC:	linux-usb@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Handshaking on USB serial devices

Greg KH wrote:
> On Thu, Feb 14, 2008 at 01:15:37AM +1030, David Newall wrote:
>   
>> Consider a USB-attached serial port that is set to do RTS/CTS (or
>> DSR/DTR) handshaking: What stops the kernel sending more data to it when
>> the remote end lowers CTS (or DTR)?
>>     
>
> The tty layer should look at the proper flags and not send data on to
> the driver in this kind of instance.
>
> Is this not happening properly for you?  If so, which USB serial driver?
>   

It's not happening properly, for pl2303 (and, I admit it! on kernel
2.4.)  I think there's a widespread problem (i.e. with other drivers.) 
None of the drivers that I've examined check CTS or DTR; or, if they do,
I can't see where.  Likewise, I can't see it being done in n_tty nor
tty_io; not even in usbserial.  The best answer I can see is write_room,
but that doesn't seem quite right.  It seems that data written (via
*_write) will merrily be transmitted to the converter, even while the
remote end is signalling to stop, even if local hardware buffers fill.

I have a working solution for the 2.4 driver, but am looking towards
something for 2.6 and beyond.  The 2.4 solution is in two parts: first,
implement a write_room function, which returns 0 when transmission is
required to stop, which apparently gives a hint to the tty layer.  (NB,
doesn't help when using a different line discipline.)  The second part
is to return 0 (or maybe -EAGAIN?) in the write function when
transmission is required to stop.  This appears sound.  (Note that the
generic putchar does no error checking, and fails when the write URB is
busy.  That's a problem with a fairly obvious solution.)

The current 2.6 driver maintains it's own buffer.  I think that's a bad
thing: usbserial already buffers writes, and the extra buffer copy seems
unnecessary, however it does solve the putchar problem.  Buffered (i.e.
by the 2.6 series pl2303 driver) data is written as soon as practicable,
regardless of CTS/DTR.  The same general workaround, but placed in
pl2303_send seems correct to me; that is, stop submitting write urbs
when the remote end lowers CTS/DTR, and trigger the resume from the
interrupt callback (specifically in update_line_status.)

As I say, this appears to be a widespread problem.  I've looked at a
number of drivers and in none of them can I see anything to prevent
transmission when the remote end demands a stop.  I'm not sure that the
tty layer, which I think I said attempts to address the problem, albeit
it does a half-arsed job, is the right place to do hardware
handshaking.  Equally, I'm not sure that the wheel should be re-invented
for every driver, but that might be unavoidable.

To make it clear: Even aside from the buffer in 2.6's pl2303.c, there's
a race: An in-flight write URB can fill all hardware buffers, making
unsafe what previously appeared to be a safe write.  I think it's
essential to delay submission of the URB on a stop-transmit condition.
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ