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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 23 Nov 2019 23:46:31 +0100 From: Heiner Kallweit <hkallweit1@...il.com> To: Brian Norris <briannorris@...omium.org> Cc: Realtek linux nic maintainers <nic_swsd@...ltek.com>, linux-kernel@...r.kernel.org, netdev@...r.kernel.org, Chun-Hao Lin <hau@...ltek.com> Subject: Re: [PATCH] [RFC] r8169: check for valid MAC before clobbering On 23.11.2019 01:51, Brian Norris wrote: > Hi Heiner, > > Thanks for the response, and sorry for some delay. I've been busy in the > last week. > > On Wed, Nov 13, 2019 at 09:30:42PM +0100, Heiner Kallweit wrote: >> On 13.11.2019 01:58, Brian Norris wrote: >>> I have some old systems with RTL8168g Ethernet, where the BIOS (based on >>> Coreboot) programs the MAC address into the MAC0 registers (at offset >>> 0x0 and 0x4). The relevant Coreboot source is publicly available here: >>> >>> https://review.coreboot.org/cgit/coreboot.git/tree/src/mainboard/google/jecht/lan.c?h=4.10#n139 >>> >>> (The BIOS is built off a much older branch, but the code is effectively >>> the same.) >>> >>> Note that this was apparently the recommended solution in an application >>> note at the time (I have a copy, but it's not marked for redistribution >>> :( ), with no mention of the method used in rtl_read_mac_address(). >>> >> The application note refers to RTL8105e which is quite different from >> RTL8168g. > > Understood. But the register mapping for this part does appear to be the > same, and I'm really having trouble finding any other documentation, so > I can't really blame whoever was writing the Coreboot code in the first > place. > >> For RTL8168g the BIOS has to write the MAC to the respective >> GigaMAC registers, see rtl_read_mac_address for these registers. > > I already see the code, but do you have any reference docs? For example, > how am I to determine "has to"? I've totally failed at finding any good > documentation. > > To the contrary, I did find an alleged RTL8169 document (no clue if it's > legit), and it appears to describe the IDR0-5 registers (i.e., offset > 0000h) as: > > ID Register 0: The ID registers 0-5 are only permitted to write by > 4-byte access. Read access can be byte, word, or double word access. > The initial value is autoloaded from EEPROM EthernetID field. > > If that implies anything, it seems to imply that any EEPROM settings > should be automatically applied, and that register 0-5h are the correct > source of truth. > > Or it doesn't really imply anything, except that some other similar IP > doesn't specifically mention this "backup register." > >> If recompiling the BIOS isn't an option, > > It's not 100% impossible, but it seems highly unlikely to happen. To me > (and likely the folks responsible for this BIOS), this looks like a > kernel regression (this driver worked just fine for me before commit > 89cceb2729c7). > On an additional note: The referenced coreboot driver is part of the Google JECHT baseboard support. Most likely the driver is just meant to support the Realtek chip version found on this board. I doubt the driver authors intended to support each and every Realtek NIC chip version. >> then easiest should be to >> change the MAC after boot with "ifconfig" or "ip" command. > > No, I think the easiest option is to apply my patch, which I'll probably > do if I can't find anything else. > > I'm curious: do you see any problem with my patch? In your > understanding, what's the purpose of the "backup registers" (as they > were called in commit 89cceb2729c7)? To be the primary source of MAC > address information? Or to only be a source if the primary registers are > empty? If the latter, then my patch should be a fine substitute. > > Brian > >>> The result is that ever since commit 89cceb2729c7 ("r8169:add support >>> more chips to get mac address from backup mac address register"), my MAC >>> address changes to use an address I never intended. >>> >>> Unfortunately, these commits don't really provide any documentation, and >>> I'm not sure when the recommendation actually changed. So I'm sending >>> this as RFC, in case I can get any tips from Realtek on how to avoid >>> breaking compatibility like this. >>> >>> I'll freely admit that the devices in question are currently pinned to >>> an ancient kernel. We're only recently testing newer kernels on these >>> devices, which brings me here. >>> >>> I'll also admit that I don't have much means to test this widely, and >>> I'm not sure what implicit behaviors other systems were depending on >>> along the way. >>> >>> Fixes: 89cceb2729c7 ("r8169:add support more chips to get mac address from backup mac address register") >>> Fixes: 6e1d0b898818 ("r8169:add support for RTL8168H and RTL8107E") >>> Cc: Chun-Hao Lin <hau@...ltek.com> >>> Signed-off-by: Brian Norris <briannorris@...omium.org> >
Powered by blists - more mailing lists