[PATCH 0/3] hwmon: coretemp fixes

[PATCH 0/3] hwmon: coretemp fixes

[Guenter Roeck]

The patches in this series fix bugs identified by Sashikoi and confirmed
in a session with gemini-4-carbon, which also implemented the fixes.

----------------------------------------------------------------
Guenter Roeck (3):
      hwmon: (coretemp) Check for NULL platform device in hotplug callbacks
      hwmon: (coretemp) Set nr_cores only after core_data allocation succeeds
      hwmon: (coretemp) Fix 32-bit jiffies wraparound in show_temp()

 drivers/hwmon/coretemp.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

[PATCH 1/3] hwmon: (coretemp) Check for NULL platform device in hotplug callbacks

[Guenter Roeck]

coretemp_get_pdev() returns NULL when topology_logical_die_id(cpu) is
negative (such as -ENODEV or -ERANGE from topology_get_logical_id() on a
firmware/hardware APIC ID mismatch) or exceeds max_zones (for example,
under asymmetric or anomalous ACPI die topologies where
topology_max_packages() * topology_max_dies_per_package() is smaller
than the logical die count).

Commit 6d03bbff456b ("hwmon: (coretemp) Simplify platform device
handling") pre-allocated zone_devices[0 .. max_zones - 1] during module
init and dropped the NULL checks on pdev in coretemp_cpu_online() and
coretemp_cpu_offline(), passing pdev unconditionally to
platform_get_drvdata(). When coretemp_get_pdev() returns NULL,
platform_get_drvdata(pdev) dereferences &pdev->dev and triggers a kernel
NULL pointer dereference.

Check for a NULL pdev and return 0 in both coretemp_cpu_online() and
coretemp_cpu_offline() before calling platform_get_drvdata(). Returning
0 in coretemp_cpu_online() ignores the unmapped CPU without blocking CPU
bringup or tearing down valid thermal zones on other dies.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 6d03bbff456b ("hwmon: (coretemp) Simplify platform device handling")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/coretemp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 3836beade59f..eb74ecd12310 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -695,6 +695,9 @@ static int coretemp_cpu_online(unsigned int cpu)
 	if (!housekeeping_cpu(cpu, HK_TYPE_MISC))
 		return 0;
 
+	if (!pdev)
+		return 0;
+
 	pdata = platform_get_drvdata(pdev);
 	if (!pdata->hwmon_dev) {
 		struct device *hwmon;
@@ -746,6 +749,9 @@ static int coretemp_cpu_offline(unsigned int cpu)
 		return 0;
 
 	/* If the physical CPU device does not exist, just return */
+	if (!pdev)
+		return 0;
+
 	pd = platform_get_drvdata(pdev);
 	if (!pd->hwmon_dev)
 		return 0;
-- 
2.45.2

Re: [PATCH 1/3] hwmon: (coretemp) Check for NULL platform device in hotplug callbacks

[sashiko-bot]

> coretemp_get_pdev() returns NULL when topology_logical_die_id(cpu) is
> negative (such as -ENODEV or -ERANGE from topology_get_logical_id() on a
> firmware/hardware APIC ID mismatch) or exceeds max_zones (for example,
> under asymmetric or anomalous ACPI die topologies where
> topology_max_packages() * topology_max_dies_per_package() is smaller
> than the logical die count).
> 
> Commit 6d03bbff456b ("hwmon: (coretemp) Simplify platform device
> handling") pre-allocated zone_devices[0 .. max_zones - 1] during module
> init and dropped the NULL checks on pdev in coretemp_cpu_online() and
> coretemp_cpu_offline(), passing pdev unconditionally to
> platform_get_drvdata(). When coretemp_get_pdev() returns NULL,
> platform_get_drvdata(pdev) dereferences &pdev->dev and triggers a kernel
> NULL pointer dereference.
> 
> Check for a NULL pdev and return 0 in both coretemp_cpu_online() and
> coretemp_cpu_offline() before calling platform_get_drvdata(). Returning
> 0 in coretemp_cpu_online() ignores the unmapped CPU without blocking CPU
> bringup or tearing down valid thermal zones on other dies.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: 6d03bbff456b ("hwmon: (coretemp) Simplify platform device handling")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031153528.3195123-1-linux@roeck-us.net?part=1

[PATCH 2/3] hwmon: (coretemp) Set nr_cores only after core_data allocation succeeds

[Guenter Roeck]

In init_temp_data(), pdata->nr_cores is populated from
topology_num_cores_per_package() before allocating pdata->core_data. If
that allocation fails, init_temp_data() returns NULL with
pdata->core_data still NULL and pdata->nr_cores set to a positive value.

coretemp_add_core() logs the error without failing coretemp_cpu_online(),
so the CPU still comes online with pdata->hwmon_dev registered. When the
CPU is later taken offline (or the module is unloaded),
coretemp_cpu_offline() calls get_temp_data(pd, cpu), which loops up to
pdata->nr_cores and dereferences the NULL pdata->core_data pointer.

Store the core count in a local variable and assign pdata->nr_cores only
after pdata->core_data is allocated, preserving the invariant that
pdata->nr_cores is zero whenever pdata->core_data is NULL.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 1a793caf6f69 ("hwmon: (coretemp) Use dynamic allocated memory for core temp_data")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/coretemp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index eb74ecd12310..7fced0e2abeb 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -491,11 +491,12 @@ init_temp_data(struct platform_data *pdata, unsigned int cpu, int pkg_flag)
 	struct temp_data *tdata;
 
 	if (!pdata->core_data) {
-		pdata->nr_cores = topology_num_cores_per_package();
-		pdata->core_data = kzalloc_objs(struct temp_data *,
-						pdata->nr_cores);
+		int nr_cores = topology_num_cores_per_package();
+
+		pdata->core_data = kzalloc_objs(struct temp_data *, nr_cores);
 		if (!pdata->core_data)
 			return NULL;
+		pdata->nr_cores = nr_cores;
 	}
 
 	tdata = kzalloc_obj(struct temp_data);
-- 
2.45.2

Re: [PATCH 2/3] hwmon: (coretemp) Set nr_cores only after core_data allocation succeeds

[sashiko-bot]

> In init_temp_data(), pdata->nr_cores is populated from
> topology_num_cores_per_package() before allocating pdata->core_data. If
> that allocation fails, init_temp_data() returns NULL with
> pdata->core_data still NULL and pdata->nr_cores set to a positive value.
> 
> coretemp_add_core() logs the error without failing coretemp_cpu_online(),
> so the CPU still comes online with pdata->hwmon_dev registered. When the
> CPU is later taken offline (or the module is unloaded),
> coretemp_cpu_offline() calls get_temp_data(pd, cpu), which loops up to
> pdata->nr_cores and dereferences the NULL pdata->core_data pointer.
> 
> Store the core count in a local variable and assign pdata->nr_cores only
> after pdata->core_data is allocated, preserving the invariant that
> pdata->nr_cores is zero whenever pdata->core_data is NULL.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: 1a793caf6f69 ("hwmon: (coretemp) Use dynamic allocated memory for core temp_data")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031153528.3195123-1-linux@roeck-us.net?part=2

[PATCH 3/3] hwmon: (coretemp) Fix 32-bit jiffies wraparound in show_temp()

[Guenter Roeck]

show_temp() rate-limits MSR reads by storing tdata->last_updated as an
unsigned long and comparing it against jiffies using
time_after(jiffies, tdata->last_updated + HZ).

time_after() relies on signed long subtraction, which only produces the
expected result while the two timestamps are within LONG_MAX jiffies of
each other. On 32-bit kernels, if tempX_input is not read for more than
LONG_MAX jiffies (~24.85 days at HZ=1000), the signed difference flips
sign and time_after() evaluates to false. Because tdata->last_updated is
only refreshed when time_after() succeeds, subsequent reads remain
blocked from refreshing for another ~24.85 days until jiffies wraps
past tdata->last_updated + HZ again, returning either a stale cached
temperature or -ENODATA if the sensor had never been read since
initialization.

Store tdata->last_updated as a u64 and use get_jiffies_64() with
time_after64(), which does not wrap in any practical uptime.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: bebe467823c0 ("hwmon: New coretemp driver")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/coretemp.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 7fced0e2abeb..96d57fe12de6 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -76,7 +76,7 @@ struct temp_data {
 	int temp;
 	bool temp_valid;
 	int tjmax;
-	unsigned long last_updated;
+	u64 last_updated;
 	unsigned int cpu;
 	int index;
 	u32 cpu_core_id;
@@ -407,7 +407,7 @@ static ssize_t show_temp(struct device *dev,
 	mutex_lock(&tdata->update_lock);
 
 	/* Check whether the time interval has elapsed */
-	if (time_after(jiffies, tdata->last_updated + HZ)) {
+	if (time_after64(get_jiffies_64(), tdata->last_updated + HZ)) {
 		tjmax = get_tjmax(tdata, dev);
 		rdmsrq_on_cpu(tdata->cpu, tdata->status_reg, &val.q);
 		/*
@@ -417,7 +417,7 @@ static ssize_t show_temp(struct device *dev,
 		 */
 		tdata->temp_valid = (val.l >> 31) & 1;
 		tdata->temp = tjmax - ((val.l >> 16) & 0xff) * 1000;
-		tdata->last_updated = jiffies;
+		tdata->last_updated = get_jiffies_64();
 	}
 
 	valid = tdata->temp_valid;
@@ -521,11 +521,8 @@ init_temp_data(struct platform_data *pdata, unsigned int cpu, int pkg_flag)
 	tdata->cpu = cpu;
 	tdata->cpu_core_id = topology_core_id(cpu);
 	tdata->attr_size = MAX_CORE_ATTRS;
-	/*
-	 * A zero timestamp does not look stale on 32-bit, where jiffies
-	 * starts just short of wrapping. Backdate it instead.
-	 */
-	tdata->last_updated = jiffies - HZ - 1;
+	/* Backdate to force a refresh on the first read */
+	tdata->last_updated = get_jiffies_64() - HZ - 1;
 	mutex_init(&tdata->update_lock);
 	return tdata;
 }
-- 
2.45.2

Re: [PATCH 3/3] hwmon: (coretemp) Fix 32-bit jiffies wraparound in show_temp()

[sashiko-bot]

> show_temp() rate-limits MSR reads by storing tdata->last_updated as an
> unsigned long and comparing it against jiffies using
> time_after(jiffies, tdata->last_updated + HZ).
> 
> time_after() relies on signed long subtraction, which only produces the
> expected result while the two timestamps are within LONG_MAX jiffies of
> each other. On 32-bit kernels, if tempX_input is not read for more than
> LONG_MAX jiffies (~24.85 days at HZ=1000), the signed difference flips
> sign and time_after() evaluates to false. Because tdata->last_updated is
> only refreshed when time_after() succeeds, subsequent reads remain
> blocked from refreshing for another ~24.85 days until jiffies wraps
> past tdata->last_updated + HZ again, returning either a stale cached
> temperature or -ENODATA if the sensor had never been read since
> initialization.
> 
> Store tdata->last_updated as a u64 and use get_jiffies_64() with
> time_after64(), which does not wrap in any practical uptime.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Fixes: bebe467823c0 ("hwmon: New coretemp driver")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031153528.3195123-1-linux@roeck-us.net?part=3

Re: [PATCH 0/3] hwmon: coretemp fixes

[Asaf Nuri Bertizlioğlu]

Tested on an Intel Core i7-14700F (8 P-cores + 12 E-cores, 1 package),
x86_64. I built coretemp.c from groeck/linux-staging hwmon branch,
commit 2b05d95c12b8, as an out-of-tree module against the running
7.3.0-4-cachyos kernel, and loaded it in place of the stock module.

Observed with the patched module:
- Loads cleanly, 21 temperature inputs (Package id 0 + 20 cores),
  values look sane in sensors.
- Offlining both SMT threads of a P-core (cpu2/cpu3) and one E-core
  (cpu27) removes exactly that core's input; onlining brings it back.
  21 inputs afterwards, same labels.
- Readings refresh after the 1 s rate limit: temp1_input went from
  32000 to 61000-63000 under stress-ng load and back to 33000 after it.
- rmmod and insmod again: 21 inputs.
- No BUG/Oops/WARNING in dmesg.

Not tested: the 32-bit jiffies wraparound (patch 3) - this is a 64-bit
system, so I can only say it shows no regression here. The NULL
platform device path (patch 1) and the core_data allocation failure
path (patch 2) are not triggered on this hardware either, so for those
it is also just "no regression".

I used an LLM to help prepare the test setup and this reply; I ran the
tests and checked the results myself.

Tested-by: Asaf Nuri Bertizlioğlu <ycistak@gmail.com>

Re: [PATCH 0/3] hwmon: coretemp fixes

[Guenter Roeck]

On 10/31/26 12:10, Asaf Nuri Bertizlioğlu wrote:
> Tested on an Intel Core i7-14700F (8 P-cores + 12 E-cores, 1 package),
> x86_64. I built coretemp.c from groeck/linux-staging hwmon branch,
> commit 2b05d95c12b8, as an out-of-tree module against the running
> 7.3.0-4-cachyos kernel, and loaded it in place of the stock module.
> 
> Observed with the patched module:
> - Loads cleanly, 21 temperature inputs (Package id 0 + 20 cores),
>    values look sane in sensors.
> - Offlining both SMT threads of a P-core (cpu2/cpu3) and one E-core
>    (cpu27) removes exactly that core's input; onlining brings it back.
>    21 inputs afterwards, same labels.
> - Readings refresh after the 1 s rate limit: temp1_input went from
>    32000 to 61000-63000 under stress-ng load and back to 33000 after it.
> - rmmod and insmod again: 21 inputs.
> - No BUG/Oops/WARNING in dmesg.
> 
> Not tested: the 32-bit jiffies wraparound (patch 3) - this is a 64-bit
> system, so I can only say it shows no regression here. The NULL
> platform device path (patch 1) and the core_data allocation failure
> path (patch 2) are not triggered on this hardware either, so for those
> it is also just "no regression".
> 
> I used an LLM to help prepare the test setup and this reply; I ran the
> tests and checked the results myself.
> 
> Tested-by: Asaf Nuri Bertizlioğlu <ycistak@gmail.com>

Thanks!

Guenter