netdev - Re: [RFC] r8152: pass through needs to be singular

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: <YuvIqCBAcZTMh0xV@lunn.ch>
Date:   Thu, 4 Aug 2022 15:24:56 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Oliver Neukum <oneukum@...e.com>
Cc:     hayeswang@...ltek.com, netdev@...r.kernel.org
Subject: Re: [RFC] r8152: pass through needs to be singular

> I am afraid I have to beg to differ.
> 
> We have a couple of related issues here. Obviously whoever designed
> MAC pass-through did not consider the case of somebody using two
> docking stations at the same time. While pass-through is used I agree
> that this is unsolvable.
> 
> Yet connecting two (or even more) docking stations to a host is
> within spec. They are USB devices (partially) and if they contain
> a NIC it is clear what we have to do.
> We would operate them with
> 1. a MAC contained on the device
> 2. if the device contains no MAC, we'd use a random MAC, but a
> different one per device. User space can assign any MAC it wants to,
> but we are talking defaults here.
> 
> Now, the question arises whether we let another feature interfere
> with that. And then I must say, if we do that and we have decided
> to take a feature that does so, we'll have to do it in a way that
> stays within spec. Yet I am sorry, you cannot give out the same
> MAC to two devices at the same time, if you want to do so.
> 
> So while the bug in the driver derives from a stupid design,
> it nevertheless is a bug and my patch fixes it. Optimally?
> No, of course not, the design is broken.

The problem is regressions. Current code will put the MAC address on
both. I guess most users just have one dock with a cabled and the
second is unused. Both will get the same MAC address, the DHCP server
will recognise the MAC address and give out the expected IP address.

If you change it to only give the MAC address out once, you get into a
race condition, which device probes first, gets the MAC, and gives the
expected MAC to the DHCP server? You are going to see regressions.

> >>> What exactly is your problem which you are trying to fix? 
> >> Adressing the comment Hayes made when reset_resume() was fixed
> >> from a deadlock, that it still assigns wrong MACs. I feel that
> >> before I fix keeping the correct address I better make sure the
> >> MAC is sane in the first place.
> > 
> > I would say that reset_resume() should restore whatever the MAC
> > address was before the suspend.
> 
> The problem is that these devices reread the passed through MAC
> at post_reset(). Do you want reset_resume() to act differently?

I would expect whatever MAC address is in the netdev structure to be
put on the interface at resume. That should of been the MAC it was
using before suspend. And by doing that, you bypass all the discussion
about where it came from.

      Andrew