netdev - Re: [PATCH v3 1/4] Simplify usbnet_cdc_update

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: <87h8lhwogm.fsf@miraculix.mork.no>
Date:   Mon, 02 Jul 2018 19:13:13 +0200
From:   Bjørn Mork <bjorn@...k.no>
To:     Oliver Neukum <oneukum@...e.com>
Cc:     Miguel Rodríguez Pérez 
        <miguel@....uvigo.gal>, gregkh@...uxfoundation.org,
        linux-usb@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v3 1/4] Simplify usbnet_cdc_update_filter

Oliver Neukum <oneukum@...e.com> writes:
> On So, 2018-07-01 at 11:05 +0200, Miguel Rodríguez Pérez         wrote:
>> Remove some unneded varibles to make the code easier to read
>> and, replace the generic usb_control_msg function for the
>> more specific usbnet_write_cmd.
>> 
>> Signed-off-by: Miguel Rodríguez Pérez <miguel@....uvigo.gal>
>
> No,
>
> sorry, but this is not good. The reason is a bit subtle.
> Drivers need to reset the filters when handling post_reset()
> [ and reset_resume() ] usbnet_write_cmd() falls back to
> kmemdup() with GFP_KERNEL. Usbnet is a framework with class
> drivers and some of the devices we drive have a storage
> interface. Thence we are on the block error handling path here.

Right.  I knew there had to be some reason this was left out when the
rest of these were converted.  But I don't think the reason is
valid... usbnet_write_cmd() will never call kmemdup when data is NULL.

There is of course also little advantage in using usbnet_write_cmd when
we don't need to allocate a buffer.  But I'd still prefer to see it for
consistency, power management and debug logging.

Or if it is left as-is: Maybe add a comment so that I don't ask the same
stupid questions in 3 weeks time? :-)  My memory is los^Husy...

> The simplest solution is to leave out this patch in the sequence.

As Miguel noted: That won't work. The switch from dev->data->control to
dev->intf is necessary to make this function reusable by other
minidrivers.

Bjørn