netdev - Re: [PATCH net-next 0/3] r8152: configuration setting

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: <8737lah6qu.fsf@miraculix.mork.no>
Date:   Thu, 08 Sep 2016 15:16:09 +0200
From:   Bjørn Mork <bjorn@...k.no>
To:     Hayes Wang <hayeswang@...ltek.com>
Cc:     "netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
        nic_swsd <nic_swsd@...ltek.com>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb\@vger.kernel.org" <linux-usb@...r.kernel.org>,
        Oliver Neukum <oliver@...kum.org>
Subject: Re: [PATCH net-next 0/3] r8152: configuration setting

Hayes Wang <hayeswang@...ltek.com> writes:

> Bjørn Mork [mailto:bjorn@...k.no]
>> Sent: Thursday, September 08, 2016 3:55 PM
> [...]
>> Yes, I see that.  But is that strictly necessary? Couldn't you just say:
>> "CDC ECM is supported by cdc_ether and therefore limited to the features
>> implemented by cdc_ether.  If you want feature X, then please use our
>> vendor specific mode with the r8152 driver?"
>
> My customs have a case that they must force the speed to 100M
> for some reasons. I also wish to implement the driver as simple
> as possible, but I don't think I could determine this. I accept
> you reject my patches. However, I couldn't deny the requests
> from the boss or customs without doing anything. I must prove
> the way is unacceptable.

That's an odd combination of requirements, but I know how customers can
be :)

Just to make it clear:  I provide comments, but I am in no position to
reject any of your patches.  Or have any wish to do so.  You maintain
r8152.  Oliver maintains cdc_ether.  I am confident that whatever you
two decide is going to be fine.

> [...]
>> Each USB configuation comes with a set of descriptors identifying the
>> functions, and USB interface drivers attach to the functions they
>> support.  The user can dynamically switch the device from e.g. cfg #1 to
>> cfg #3 by writing "3" to /sys/bus/usb/devices/<port>/bConfigurationValue
>> This will cause the ECM and ACM USB interfaces to disappear, and the
>> associated class drivers will unbind, and new vendor specific USB
>> interfaces appear instead, causing a matching vendor specific driver to
>> load and bind.
>> 
>> Naturally, end users will not switch configurations all the time.  They
>> will select the configuration providing the set of functions they want.
>> If this is different from the default configuration  selected by the
>> Linux USB core, then that's a simple udev rule to update the
>> bConfigurationValue sysfs attribute on device disceovery.
>
> I tested above method before. And I found that the cdc_ether
> was loaded before switching the configuration. The behavior
> of loading one driver and changing to another driver has
> opportunity to let our some previous chips become abnormal.
> To switch configuration is fine. However, it may have problem
> to switch driver. That is why the current kernel only supports
> vendor mode. If the method works fine, I have no trouble now.

Yes, many firmwares/devices are not prepared to do "late" config
switching and can end up in a strange limbo if they are initialized
before switching. An udev rule should still run early enough to prevent
this problem I believe.

But if that doesn't work, then I agree that a blacklist makes more
sense. Just make it runtime configurable so that distros can do
something sane with it.



Bjørn