linux-kernel - Re: [PATCH] USB: serial: add nt124 usb to serial driver

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] [day] [month] [year] [list]

Message-ID: <20141216125259.GM6778@localhost>
Date:	Tue, 16 Dec 2014 13:52:59 +0100
From:	Johan Hovold <johan@...nel.org>
To:	George McCollister <george.mccollister@...il.com>
Cc:	Johan Hovold <johan@...nel.org>, linux-usb@...r.kernel.org,
	open list <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] USB: serial: add nt124 usb to serial driver

On Mon, Dec 15, 2014 at 10:09:22AM -0600, George McCollister wrote:
> On Mon, Dec 15, 2014 at 3:52 AM, Johan Hovold <johan@...nel.org> wrote:
> > On Sun, Dec 14, 2014 at 11:51:11AM -0600, George McCollister wrote:
> >> Johan,
> >>
> >> While working on the tx_empty changes you suggested it occurred to me
> >> that it might not be obvious to others that the firmware doesn't send
> >> a packet with the NT124_CTRL_TXEMPTY flag cleared when it begins
> >> transmitting. The practical implication is that if the driver sets
> >> tx_empty = true when it sees NT124_CTRL_TXEMPTY, tx_empty must be
> >> reset to false somewhere when more data is transmitted. Perhaps I
> >> could add prepare_write_buffer and do it in there before calling
> >> usb_serial_generic_prepare_write_buffer(). Does that sound acceptable?
> >
> > Hmm. There's no way to query that flag? And the status is sent (as bulk
> > in data) periodically or only when data has been received? And not when
> > the actual status changes?
>
> The bulk in packets are not sent periodically only on TXEMPTY, other
> line change or received data. There's no way to query the flag, though
> we're still at the stage we can make modifications to the firmware if
> there's justification. One of the design goals is to minimize
> unnecessary USB traffic so if there's a place to clear the flag in the
> driver without querying it would be preferable.
>
> > A potential problem with using prepare_write_buffer would be on failures
> > to submit the write urb, in which case this flag might never be cleared.
>
> Yes, this could be a problem though I wonder how many (if any)
> recoverable situations this would happen in that didn't eventually
> lead to a transmission. Adding a timer with a long timeout that set
> tx_empty  = true crossed my mind but that doesn't really seem any less
> error prone than the original issue. I could of course duplicate a
> bunch of code from generic.c and make a few minor changes to set
> tx_empty = true but that obviously isn't desirable either.
> 
> If none of the above solutions sound acceptable, let me know I and I
> guess I'll change the firmware to allow polling of TXEMPTY with a
> control message and remove it from the bulk in packet.

I think it would be best if you could add a way to query the driver. It
seems like that is the only way to avoid having write race with the
tx_empty bulk-in status reports.

Thanks,
Johan
--
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/