netdev - Re: [PATCH v2] net: usb: r8152: Add in new Devices that are supported for Mac-Passthru

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: <874k1wdv5j.fsf@miraculix.mork.no>
Date:   Wed, 11 May 2022 16:02:48 +0200
From:   Bjørn Mork <bjorn@...k.no>
To:     David Ober <dober6023@...il.com>
Cc:     linux-usb@...r.kernel.org, netdev@...r.kernel.org,
        davem@...emloft.net, hayeswang@...ltek.com, aaron.ma@...onical.com,
        mpearson@...ovo.com, dober@...ovo.com
Subject: Re: [PATCH v2] net: usb: r8152: Add in new Devices that are
 supported for Mac-Passthru

David Ober <dober6023@...il.com> writes:

> Lenovo Thunderbolt 4 Dock, and other Lenovo USB Docks are using the original
> Realtek USB ethernet Vendor and Product IDs
> If the Network device is Realtek verify that it is on a Lenovo USB hub
> before enabling the passthru feature
>
> This also adds in the device IDs for the Lenovo USB Dongle and one other
> USB-C dock
>
> Signed-off-by: David Ober <dober6023@...il.com>
> ---
>  drivers/net/usb/r8152.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index c2da3438387c..c32b9bf90baa 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -771,6 +771,9 @@ enum rtl8152_flags {
>  };
>  
>  #define DEVICE_ID_THINKPAD_THUNDERBOLT3_DOCK_GEN2	0x3082
> +#define DEVICE_ID_THINKPAD_THUNDERBOLT4_DOCK_GEN1	0x8153


We used to have a macro named PRODUCT_ID_RTL8153 for this magic number,
but it was removed in 2014:

commit 662412d14bfa6a672626e4470cab73b75c8b42f0
Author: hayeswang <hayeswang@...ltek.com>
Date:   Thu Nov 6 12:47:40 2014 +0800

    r8152: remove the definitions of the PID
    
    The PIDs are only used in the id table, so the definitions are
    unnacessary. Remove them wouldn't have confusion.
    
    Signed-off-by: Hayes Wang <hayeswang@...ltek.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index cf1b8a7a4c77..66b139a8b6ca 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -461,11 +461,7 @@ enum rtl8152_flags {
 
 /* Define these values to match your device */
 #define VENDOR_ID_REALTEK              0x0bda
-#define PRODUCT_ID_RTL8152             0x8152
-#define PRODUCT_ID_RTL8153             0x8153
-
 #define VENDOR_ID_SAMSUNG              0x04e8
-#define PRODUCT_ID_SAMSUNG             0xa101
 
 #define MCU_TYPE_PLA                   0x0100
 #define MCU_TYPE_USB                   0x0000
@@ -3898,9 +3894,9 @@ static void rtl8152_disconnect(struct usb_interface *intf)
 
 /* table of devices that work with this driver */
 static struct usb_device_id rtl8152_table[] = {
-       {USB_DEVICE(VENDOR_ID_REALTEK, PRODUCT_ID_RTL8152)},
-       {USB_DEVICE(VENDOR_ID_REALTEK, PRODUCT_ID_RTL8153)},
-       {USB_DEVICE(VENDOR_ID_SAMSUNG, PRODUCT_ID_SAMSUNG)},
+       {USB_DEVICE(VENDOR_ID_REALTEK, 0x8152)},
+       {USB_DEVICE(VENDOR_ID_REALTEK, 0x8153)},
+       {USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101)},
        {}
 };
 


Re-introducing it as  DEVICE_ID_THINKPAD_THUNDERBOLT4_DOCK_GEN1 is
confusing/obfuscating in several ways:

 - the same value is now used two places in the driver, but only one of
   those places use the macro
  
 - the name indicates that this is somehow unique to a specific Thinkpad
   product, which it obviously isn't.  It's one of the most common
   device IDs for ethernet USB dongles at the moment, used by any number
   of vendors

 - the attempt to treat these devices differently based on the parent
   vendor will cause confusion for anyone connecting any of these
   dongles to a Lenovo hub.  This will match so much more than one
   specific dock product


I beleive I've said this before, but these policies would have been much
better handled in userspace with the system mac address being a resource
made available by some acpi driver. But whatever.  I look forward to
seeing the FCC unlock logic for Lenovo X55 modems added to the
drivers/bus/mhi/host/pci_generic.c driver.

  
Bjørn