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
| ||
|
Message-ID: <BYAPR11MB35583CCCF0C908763B5BA5E3EC489@BYAPR11MB3558.namprd11.prod.outlook.com> Date: Wed, 31 May 2023 22:43:01 +0000 From: <Tristram.Ha@...rochip.com> To: <andrew@...n.ch> CC: <davem@...emloft.net>, <f.fainelli@...il.com>, <netdev@...r.kernel.org>, <UNGLinuxDriver@...rochip.com> Subject: RE: [PATCH net-next] net: phy: smsc: add WoL support to LAN8740/LAN8742 PHYs. > Subject: Re: [PATCH net-next] net: phy: smsc: add WoL support to LAN8740/LAN8742 > PHYs. > > > + if (wol->wolopts & WAKE_ARP) { > > + const u8 *ip_addr = > > + ((const u8 *)&((ndev->ip_ptr)->ifa_list)->ifa_address); > > I'm not sure this is safe. What happens when the interface only has an > IPv6 address? Is ifa_list a NULL pointer? I really think you need to > be using a core helper to get the IPv4 address. > This will be fixed with in_dev_get() and rcu_dereference(). Indeed the address will disappear when the link is down. Why is that so? > > + const u16 mask[3] = { 0xF03F, 0x003F, 0x03C0 }; > > Are there any endianness issues here? I've not looked at how mask is > used, but if it is indicating which bytes in the pattern should be > matched on, i guess endian does matter. These values eventually are programmed into MMD registers. There are 8 for 128-bit value. Bit 0 in the last register is byte 0. I do not see PHY register values are checked for endianness. > > + u8 pattern[42] = { > > + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, > > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > + 0x08, 0x06, > > + 0x00, 0x01, 0x08, 0x00, 0x06, 0x04, 0x00, 0x01, > > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > + 0x00, 0x00, 0x00, 0x00, > > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > + 0x00, 0x00, 0x00, 0x00 }; > > > > + if (wol->wolopts & WAKE_MCAST) { > > + u8 pattern[6] = { 0x33, 0x33, 0xFF, 0x00, 0x00, 0x00 }; > > + u16 mask[1] = { 0x0007 }; > > + u8 len = 3; > > + > > + /* Try to match IPv6 Neighbor Solicitation. */ > > + if (ndev->ip6_ptr) { > > + struct list_head *addr_list = > > + &ndev->ip6_ptr->addr_list; > > + struct inet6_ifaddr *ifa; > > + > > + list_for_each_entry(ifa, addr_list, if_list) { > > + if (ifa->scope == IFA_LINK) { > > + memcpy(&pattern[3], > > + &ifa->addr.in6_u.u6_addr8[13], > > + 3); > > + mask[0] = 0x003F; > > + len = 6; > > + break; > > + } > > + } > > + } > > From an architecture point of view, i don't think a PHY driver should > be access these data structure directly. See if ipv6_get_lladdr() does > what you need? ipv6_get_lladdr() is not exported so cannot be used when building the PHY driver as a module. The WAKE_ARP and WAKE_MCAST code just want a regular IPv4 address and an IPv6 address as shown by ifconfig command. They are more like an example to show how the hardware pattern filtering is done. > > + if (wol->wolopts & (WAKE_MAGIC | WAKE_UCAST)) { > > + const u8 *mac = (const u8 *)ndev->dev_addr; > > + > > + if (!is_valid_ether_addr(mac)) > > + return -EINVAL; > > Is that possible? Does the hardware care? > Removed.
Powered by blists - more mailing lists