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: <AM6PR05MB5224D8338CD23225746CBA72A2670@AM6PR05MB5224.eurprd05.prod.outlook.com> Date: Thu, 14 Feb 2019 07:06:32 +0000 From: Vadim Pasternak <vadimp@...lanox.com> To: Guenter Roeck <linux@...ck-us.net>, Andrew Lunn <andrew@...n.ch>, Ido Schimmel <idosch@...lanox.com> CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "davem@...emloft.net" <davem@...emloft.net>, Jiri Pirko <jiri@...lanox.com>, mlxsw <mlxsw@...lanox.com> Subject: RE: [PATCH net-next 09/12] mlxsw: core: Extend hwmon interface with fan fault attribute > -----Original Message----- > From: Guenter Roeck <groeck7@...il.com> On Behalf Of Guenter Roeck > Sent: Wednesday, February 13, 2019 5:03 PM > To: Andrew Lunn <andrew@...n.ch>; Ido Schimmel <idosch@...lanox.com> > Cc: netdev@...r.kernel.org; davem@...emloft.net; Jiri Pirko > <jiri@...lanox.com>; mlxsw <mlxsw@...lanox.com>; Vadim Pasternak > <vadimp@...lanox.com> > Subject: Re: [PATCH net-next 09/12] mlxsw: core: Extend hwmon interface with > fan fault attribute > > On 2/13/19 5:53 AM, Andrew Lunn wrote: > > On Wed, Feb 13, 2019 at 11:28:53AM +0000, Ido Schimmel wrote: > >> From: Vadim Pasternak <vadimp@...lanox.com> > >> > >> Add new fan hwmon attribute for exposing fan faults (fault indication > >> is read from Fan Out of Range Event Register). > >> > >> Signed-off-by: Vadim Pasternak <vadimp@...lanox.com> > >> Reviewed-by: Jiri Pirko <jiri@...lanox.com> > >> Signed-off-by: Ido Schimmel <idosch@...lanox.com> > > > > Hi Ido > > > > You should include the HWMON maintainer in the Cc: list. > > > > I would not be too surprised if he says to use > > hwmon_device_register_with_info(). > > > > I would ask to do that for new drivers, but this is is not a new driver. > On top of that, I wasn't included in its initial review. Since I wasn't involved, I > have no idea what shape the driver is in, and for sure won't review it now (to > retain my sanity). > > Only comment I have is that using the _with_info API and using devm_ functions > might simplify the driver a lot. I'll be happy to do a review if/when that is done. > > Guenter Hi Guenter, Thank you for your comments. But, this patch adds one new FAN attribute to the existing infrastructure. At the time mlxsw core_hwmon module has been submitted, the API hwmon_device_register_with_info() was not available yet. Of course in a new modules we are using this API, like in our mlxreg-fan for example. The same is relevant for the patch 10/12 from this series – it also extends the existing infrastructure. But there, even in case of a new code the config structure for hwmon_device_register_with_info() would be look like: { /* 3 entries like below - for chip ambient temperatures (could be from 1 to 3. */ HWMON_F_INPUT | HWMON_T_HIGHEST | HWMON_T_RESET_HISTORY, ... /* 128 entries like below - for modules temperatures (could be from 16 to 128. */ HWMON_F_INPUT | HWMON_T_CRIT | HWMON_T_EMERGENCY, HWMON_T_FAULT | HWMON_T_LABEL, ... /* 64 entries like below - for interconnects temperatures (could be from 0 to 64). */ HWMON_F_INPUT | HWMON_T_HIGHEST | HWMON_T_RESET_HISTORY | HWMON_T_LABEL, 0 }; Means we'll have 195 lines for configuration description and in case future systems will be equipped with the bigger number of port slots and/or with the bigger number of interconnects devices, the below structure will require modification. Modification will be not necessary, in case we'll keep configuration like it is now. Regarding devm_ API - it has been used initially in core_hwmon module, but the in commit (9b3bc7db759e) it has been removed. And it was a good reason for that. Chip can be re-programmed after initialization in case driver determines it needs to flash a new firmware version. In such case after re-programming driver will make registration again and among the other stuff it will make new registration with hwmon subsystem. And in case it 's registered with hwmon subsystem using devm_hwmon_device_register_with_groups(), the old hwmon device (registered before the flashing) was never unregistered and was referencing stale data, resulting in a use-after free.
Powered by blists - more mailing lists