linux-kernel - Re: [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87txbnbmgv.fsf@nemi.mork.no>
Date:	Tue, 25 Feb 2014 09:55:28 +0100
From:	Bjørn Mork <bjorn@...k.no>
To:	Keith Packard <keithp@...thp.com>
Cc:	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	Kyle McMartin <kyle@...radead.org>
Subject: Re: [PATCH] net/usb: Add Lenovo ThinkPad OneLink GigaLAN USB ID to ax88179 driver

Keith Packard <keithp@...thp.com> writes:

> Kyle McMartin says:
>
>>  &lenovo_info, surely.
>
> Yeah, Daniel Stone caught the same bug. Not a big deal; the only
> difference between any of these elements is the string produced by the
> kernel.

Yes, and that mess should really be cleaned up in this driver (and
probably others).

The .description field describes the *driver*, not the device.  The
device is described by the manufacturer, using the iManufacturer,
iProduct and iSerialstring descriptors, which the USB core takes care of
logging and storing.  There is absolutely no reason to add additional,
possibly conflicting, product descriptions in these drivers.

Some additional static data does of course not matter much, but the
additional code is really an unnecessary source of subtle bugs.  I'm
pretty sure the entry you copied has bugs in the .flags field
(ax88179_rx_fixup clones the skb and returns 1, which means that
FLAG_MULTI_PACKET should be set).  Keeping numerous copies of the exact
same driver definition is just waiting for someone to forget
syncronizing any fixups for such bugs.

I propose replacing samsung_info, lenovo_info and the other entries
differing only by description with a generic driver entry, and trust the
vendors to add the appropriate string descriptors to their devices.

The separate generic entries for USB2 and USB3 versions of the chip is
also more confusing than helpful IMHO, given that the driver really is
identical. I don't think it helps anyone to see "USB 3.0 Gigabit
Ethernet" reported if the adapter is connected to a USB 2.0 host and
vice versa.  Something lik "ASIX AX88179/178A" would do just fine as
description regardless of device ID.

Bjørn
--
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/