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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z4_XQQ0tkD1EkOJ4@shell.armlinux.org.uk>
Date: Tue, 21 Jan 2025 17:20:01 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Armin Wolf <W_Armin@....de>, Huisong Li <lihuisong@...wei.com>,
	linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-media@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	arm-scmi@...r.kernel.org, netdev@...r.kernel.org,
	linux-rtc@...r.kernel.org, oss-drivers@...igine.com,
	linux-rdma@...r.kernel.org, platform-driver-x86@...r.kernel.org,
	linuxarm@...wei.com, jdelvare@...e.com, kernel@...davale.org,
	pauk.denis@...il.com, james@...iv.tech, sudeep.holla@....com,
	cristian.marussi@....com, matt@...ostay.sg, mchehab@...nel.org,
	irusskikh@...vell.com, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	saeedm@...dia.com, leon@...nel.org, tariqt@...dia.com,
	louis.peens@...igine.com, hkallweit1@...il.com, kabel@...nel.org,
	hdegoede@...hat.com, ilpo.jarvinen@...ux.intel.com,
	alexandre.belloni@...tlin.com, krzk@...nel.org,
	jonathan.cameron@...wei.com, zhanjie9@...ilicon.com,
	zhenglifeng1@...wei.com, liuyonglong@...wei.com
Subject: Re: [PATCH v1 00/21] hwmon: Fix the type of 'config' in struct
 hwmon_channel_info to u64

On Tue, Jan 21, 2025 at 06:50:00AM -0800, Guenter Roeck wrote:
> On 1/21/25 06:12, Armin Wolf wrote:
> > Am 21.01.25 um 07:44 schrieb Huisong Li:
> > > The hwmon_device_register() is deprecated. When I try to repace it with
> > > hwmon_device_register_with_info() for acpi_power_meter driver, I found that
> > > the power channel attribute in linux/hwmon.h have to extend and is more
> > > than 32 after this replacement.
> > > 
> > > However, the maximum number of hwmon channel attributes is 32 which is
> > > limited by current hwmon codes. This is not good to add new channel
> > > attribute for some hwmon sensor type and support more channel attribute.
> > > 
> > > This series are aimed to do this. And also make sure that acpi_power_meter
> > > driver can successfully replace the deprecated hwmon_device_register()
> > > later.
> 
> This explanation completely misses the point. The series tries to make space
> for additional "standard" attributes. Such a change should be independent
> of a driver conversion and be discussed on its own, not in the context
> of a new driver or a driver conversion.

I think something needs to budge here, because I think what you're
asking is actually impossible!

One can't change the type of struct hwmon_channel_info.config to be a
u64 without also updating every hwmon driver that assigns to that
member.

This is not possible:

 struct hwmon_channel_info {
         enum hwmon_sensor_types type;
-        const u32 *config;
+        const u64 *config;
 };

static u32 marvell_hwmon_chip_config[] = {
...
};

static const struct hwmon_channel_info marvell_hwmon_chip = {
        .type = hwmon_chip,
        .config = marvell_hwmon_chip_config,
};

This assignment to .config will cause a warning/error with the above
change. If instead we do:

-	.config = marvell_hwmon_chip_config,
+	.config = (u64 *)marvell_hwmon_chip_config,

which would have to happen to every driver, then no, this also doesn't
work, because config[1] now points beyond the bounds of
marvell_hwmon_chip_config, which only has two u32 entries.

You can't just change the type of struct hwmon_channel_info.config
without patching every driver that assigns to
struct hwmon_channel_info.config as things currently stand.

The only way out of that would be:

1. convert *all* drivers that defines a config array to be defined by
   their own macro in hwmon.h, and then switch that macro to make the
   definitions be a u64 array at the same time as switching struct
    hwmon_channel_info.config

2. convert *all* drivers to use HWMON_CHANNEL_INFO() unconditionally,
   and switch that along with struct hwmon_channel_info.config.

3. add a new member to struct hwmon_channel_info such as
   "const u64 *config64" and then gradually convert drivers to use it.
   Once everyone is converted over, then remove "const u32 *config",
   optionally rename "config64" back to "config" and then re-patch all
   drivers. That'll be joyful, with multiple patches to drivers that
   need to be merged in sync with hwmon changes - and last over several
   kernel release cycles.

This is not going to be an easy change!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ