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: Thu, 31 Mar 2022 10:11:22 -0700 From: Guenter Roeck <linux@...ck-us.net> To: "Russell King (Oracle)" <linux@...linux.org.uk>, Michael Walle <michael@...le.cc> Cc: Xu Yilun <yilun.xu@...el.com>, David Laight <David.Laight@...lab.com>, Tom Rix <trix@...hat.com>, Jean Delvare <jdelvare@...e.com>, Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org, netdev@...r.kernel.org Subject: Re: [PATCH v2 1/5] hwmon: introduce hwmon_sanitize_name() On 3/31/22 07:58, Russell King (Oracle) wrote: > On Thu, Mar 31, 2022 at 04:51:47PM +0200, Michael Walle wrote: >> Am 2022-03-31 16:45, schrieb Russell King (Oracle): >>> On Wed, Mar 30, 2022 at 08:23:35AM -0700, Guenter Roeck wrote: >>>> Michael, let's just drop the changes outside drivers/hwmon from >>>> the series, and let's keep hwmon_is_bad_char() in the include file. >>>> Let's just document it, explaining its use case. >>> >>> Why? There hasn't been any objection to the change. All the discussion >>> seems to be around the new function (this patch) rather than the actual >>> conversions in drivers. >>> >>> I'm entirely in favour of cleaning this up - it irks me that we're doing >>> exactly the same cleanup everywhere we have a hwmon. >>> >>> At the very least, I would be completely in favour of keeping the >>> changes in the sfp and phy code. >> >> FWIW, my plan was to send the hwmon patches first, by then my other >> series (the polynomial_calc() one) will also be ready to be picked. >> Then I'd ask Guenter for a stable branch with these two series which >> hopefully get merged into net-next. Then I can repost the missing >> patches on net-next along with the new sensors support for the GPY >> and LAN8814 PHYs. > > Okay, that's fine. It just sounded like the conversion of other drivers > outside drivers/hwmon was being dropped. > Not dropped, just disconnected. From hwmon perspective, we want a certain set of characters to be dropped or replaced. Also, from hwmon perspective, it makes sense to have the helper function allocate the replacement data. There was disagreement about which characters should be replaced, and if the helper function should allocate the replacement string or not. I have no intention to change the set of characters without good reason, and I feel quite strongly about allocating the replacement in the helper function. Since potential callers don't _have_ to use the helper and don't _have_ to provide valid names (and are responsible for the consequences), I would like that discussion to be separate from hwmon changes. > Note that there's another "sanitisation" of hwmon names in > drivers/net/phy/marvell.c - that converts any non-alnum character to > an underscore. Not sure why the different approach was chosen there. > It actually drops non-alphanumeric characters. The name is derived from the phy device name, which I think is derived from the name field in struct phy_driver. That includes spaces and '(', ')'. I honestly have no idea what libsensors would do with '(' and ')'. Either case, even if that would create a hiccup in libsensors and we would add '(' and ')' to the 'forbidden' list of characters, the fact that the code doesn't replace but drop non-alphanumeric characters means it won't be able to use a helper anyway since that would result in a hwmon 'name' attribute change and thus not be backward compatible. Besides, "Marvell_88E1111__Finisar_" would look a bit odd anyway, and "Marvell88E1111Finisar" may be at least slightly better. Guenter
Powered by blists - more mailing lists