linux-kernel - Re: [patch V2 03/13] USB: serial: keyspan

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]

Date:   Sun, 25 Oct 2020 18:05:45 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Johan Hovold <johan@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org,
        Thomas Winischhofer <thomas@...ischhofer.net>,
        "Ahmed S. Darwish" <a.darwish@...utronix.de>,
        Mathias Nyman <mathias.nyman@...el.com>,
        Valentina Manea <valentina.manea.m@...il.com>,
        Shuah Khan <shuah@...nel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        linux-omap@...r.kernel.org, Kukjin Kim <kgene@...nel.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        linux-arm-kernel@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org, Felipe Balbi <balbi@...nel.org>,
        Duncan Sands <duncan.sands@...e.fr>
Subject: Re: [patch V2 03/13] USB: serial: keyspan_pda: Consolidate room query

On Mon, Oct 19, 2020 at 12:06:32PM +0200, Thomas Gleixner wrote:
> From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> 
> Having two copies of the same code doesn't make the code more readable and
> allocating a buffer of 1 byte for a synchronous operation is a pointless
> exercise.

As Alan pointed out, this buffer is in fact required and not pointless
at all even if reallocating it may be suboptimal.

Note however that there are several further allocations done by
usb_control_msg() for each control request.

> Allocate a byte buffer at init which can be used instead. The buffer is
> only used in open() and tty->write(). Console writes are not calling into
> the query. open() obviously happens before write() and the writes are
> serialized by bit 0 of port->write_urbs_free which protects also the
> transaction itself.

As I mentioned in my comments to the previous patch, I've rewritten the
driver so that is no longer does this query in the write path. I kept
the buffer allocation for now though in case you want to rework this one
top.

Johan