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: <96800001-5d19-4b48-b43e-0cfbeccb48c1@lunn.ch> Date: Sat, 7 Oct 2023 16:47:55 +0200 From: Andrew Lunn <andrew@...n.ch> To: Trevor Gross <tmgross@...ch.edu> Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, miguel.ojeda.sandonis@...il.com, greg@...ah.com Subject: Re: [PATCH v2 1/3] rust: core abstractions for network PHY drivers > > + /// Sets the speed of the PHY. > > + pub fn set_speed(&mut self, speed: u32) { > > + let phydev = self.0.get(); > > + // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`. > > + unsafe { (*phydev).speed = speed as i32 }; > > + } > > Since we're taking user input, it probably doesn't hurt to do some > sort of sanity check rather than casting. Maybe warn once then return > the biggest nowrapping value After reading the thread, we first have a terminology problem. In the kernel world, 'user input' generally means from user space. And user space should never be trusted, but user space should also not be allowed to bring the system to its knees. Return -EINVAL to userspace is the correct thing to do and keep going. Don't do a kernel splat because the user passed 42 as a speed, not 10. However, what Trevor was meaning is that whoever called set_speed() passed an invalid value. But what are valid values? We have this file which devices the KAPI https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/ethtool.h#L1883 says: /* The forced speed, in units of 1Mb. All values 0 to INT_MAX are legal. and we also have #define SPEED_UNKNOWN -1 and there is a handy little helper: static inline int ethtool_validate_speed(__u32 speed) { return speed <= INT_MAX || speed == (__u32)SPEED_UNKNOWN; } so if you want to validate speed, call this helper. However, this is a kernel driver, and we generally trust kernel drivers. There is not much we can do except trust them. And passing an invalid speed here is very unlikely to cause the kernel to explode sometime later. Andrew
Powered by blists - more mailing lists