Skip to contentSkip to conversation

char: hpet: prevent hard-IRQ divide-by-zero in hpet_interrupt() via HPET_IRQFREQ

PATCH Superseded Hui Peng 3 participants 4 messages Sep 19 – Sep 21

Subsystems (3)
lore
Share & tools
lore

Hui Peng

author
#0 lore raw

PATCH 1/1 char: hpet: prevent hard-IRQ divide-by-zero in hpet_interrupt() via HPET_IRQFREQ

In hpet_ioctl_common(), HPET_IRQFREQ sets:

        devp->hd_ireqfreq = hpet_time_div(hpetp, arg);

where hpet_time_div() computes (hpetp->hp_tick_freq + (arg >> 1)) / arg. Whenever arg > 2 * hpetp->hp_tick_freq, integer division yields 0 and sets devp->hd_ireqfreq = 0. In addition, HPET_IRQFREQ allows updating devp->hd_ireqfreq while timer interrupts (HPET_IE) are already enabled.

When the armed HPET timer interrupt fires, hpet_interrupt() executes:

        base = mc % devp->hd_ireqfreq;

in hard-IRQ context with devp->hd_ireqfreq == 0, triggering an immediate divide error (#DE) kernel Oops.

Reject HPET_IRQFREQ with -EBUSY when HPET_IE is already enabled, and reject arg values where hpet_time_div(hpetp, arg) == 0 with -EINVAL.

Fixes ba3f213f8a31 ("[PATCH] HPET: make frequency calculations 32 bit safe")
Assisted-by LLM

---

drivers/char/hpet.c +6 -1
@@ -625,13 +625,18 @@ hpet_ioctl_common(struct hpet_dev *devp, unsigned int cmd, unsigned long arg,
 		devp->hd_flags &= ~HPET_PERIODIC;
 		break;
 	case HPET_IRQFREQ:
+		if (devp->hd_flags & HPET_IE) {
+			err = -EBUSY;
+			break;
+		}
+
 		if ((arg > hpet_max_freq) &&
 		    !capable(CAP_SYS_RESOURCE)) {
 			err = -EACCES;
 			break;
 		}
 
-		if (!arg) {
+		if (!arg || !hpet_time_div(hpetp, arg)) {
 			err = -EINVAL;
 			break;
 		}
Signature · 1 line
2.43.0
#1 lore raw
Quoted 16 lines · Hui Peng

In hpet_ioctl_common(), HPET_IRQFREQ sets:

        devp->hd_ireqfreq = hpet_time_div(hpetp, arg);

where hpet_time_div() computes (hpetp->hp_tick_freq + (arg >> 1)) / arg. Whenever arg > 2 * hpetp->hp_tick_freq, integer division yields 0 and sets devp->hd_ireqfreq = 0. In addition, HPET_IRQFREQ allows updating devp->hd_ireqfreq while timer interrupts (HPET_IE) are already enabled.

When the armed HPET timer interrupt fires, hpet_interrupt() executes:

        base = mc % devp->hd_ireqfreq;

in hard-IRQ context with devp->hd_ireqfreq == 0, triggering an immediate divide error (#DE) kernel Oops.

Quoted 7 lines

Reject HPET_IRQFREQ with -EBUSY when HPET_IE is already enabled, and reject arg values where hpet_time_div(hpetp, arg) == 0 with -EINVAL.

Fixes ba3f213f8a31 ("[PATCH] HPET: make frequency calculations 32 bit safe")
Assisted-by LLM

---

How was this tested?

Why no cc: stable?

Please slow down.

greg k-h

author
#2 lore raw

PATCH v2 1/1 char: hpet: prevent hard-IRQ divide-by-zero in hpet_interrupt() via HPET_IRQFREQ

In reply to Hui Peng

In hpet_ioctl_common(), HPET_IRQFREQ computes the timer period as:

  devp->hd_ireqfreq = hpet_time_div(hpetp, arg);

where hpet_time_div() returns div64_ul(hpetp->hp_tick_freq + (arg >> 1), arg). Whenever arg > 2 * hpetp->hp_tick_freq, integer division truncates to 0 and stores devp->hd_ireqfreq = 0.

Although hpet_ioctl_ieon() (HPET_IE_ON) checks if (!devp->hd_ireqfreq) before enabling the timer interrupt, HPET_IRQFREQ neither rejects updates while HPET_IE is already active nor checks whether hpet_time_div(hpetp, arg) evaluates to 0. As a result, arming the timer with a valid frequency (for example, HPET_IRQFREQ with 1000 Hz followed by HPET_IE_ON) and then calling HPET_IRQFREQ with a large frequency (such as 0xffffffffUL) overwrites devp->hd_ireqfreq with 0 while the timer interrupt is active. When the next interrupt fires, hpet_interrupt() reads t = devp->hd_ireqfreq (0) and computes base = mc % t, crashing the kernel in hard-IRQ context:

  Oops: divide error: 0000 [#1] SMP KASAN PTI
  CPU: 0 UID: 0 PID: 0 Comm: swapper/0
  RIP: 0010:hpet_interrupt+0x20f/0x360
  Call Trace:
   <IRQ>
   __handle_irq_event_percpu+0x102/0x400
   handle_irq_event+0xa6/0x1c0
   handle_level_irq+0x205/0x5e0
   __common_interrupt+0x60/0x130
   common_interrupt+0x7a/0x90
   </IRQ>
  Kernel panic - not syncing: Fatal exception in interrupt

Reject HPET_IRQFREQ with -EBUSY when HPET_IE is set in devp->hd_flags, and return -EINVAL when hpet_time_div(hpetp, arg) evaluates to 0.

Tested in QEMU (-global hpet.hpet-intcap=0x0c24 with noapic) by opening /dev/hpet and calling ioctl(fd, HPET_IRQFREQ, 1000), ioctl(fd, HPET_IE_ON, 0), and ioctl(fd, HPET_IRQFREQ, 0xffffffffUL): on the unfixed kernel this immediately triggers the divide error panic in hpet_interrupt(), whereas on the fixed kernel HPET_IRQFREQ returns -EBUSY while HPET_IE is enabled and -EINVAL when hpet_time_div(hpetp, arg) is 0.

Fixes ba3f213f8a31 ("[PATCH] HPET: make frequency calculations 32 bit safe")
Fixes 273ef9509b79 ("drivers/char/hpet.c: fix periodic-emulation for delayed interrupts")
Assisted-by LLM

---
Changes in v2:
- Add Cc: [email protected] and include the QEMU test procedure and oops trace in the commit description per Greg Kroah-Hartman.
- Add Fixes: 273ef9509b79 ("drivers/char/hpet.c: fix periodic-emulation for delayed interrupts") for the mc % t division in hpet_interrupt().

 drivers/char/hpet.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/char/hpet.c +6 -1
index 285c6037417a..ada95872e148 100644
@@ -625,13 +625,18 @@ hpet_ioctl_common(struct hpet_dev *devp, unsigned int cmd, unsigned long arg,
 		devp->hd_flags &= ~HPET_PERIODIC;
 		break;
 	case HPET_IRQFREQ:
+		if (devp->hd_flags & HPET_IE) {
+			err = -EBUSY;
+			break;
+		}
+
 		if ((arg > hpet_max_freq) &&
 		    !capable(CAP_SYS_RESOURCE)) {
 			err = -EACCES;
 			break;
 		}
 
-		if (!arg) {
+		if (!arg || !hpet_time_div(hpetp, arg)) {
 			err = -EINVAL;
 			break;
 		}
Signature · 1 line
2.49.0
#3 lore raw
Quoted 54 lines · Hui Peng

In hpet_ioctl_common(), HPET_IRQFREQ computes the timer period as:

  devp->hd_ireqfreq = hpet_time_div(hpetp, arg);

where hpet_time_div() returns div64_ul(hpetp->hp_tick_freq + (arg >> 1), arg). Whenever arg > 2 * hpetp->hp_tick_freq, integer division truncates to 0 and stores devp->hd_ireqfreq = 0.

Although hpet_ioctl_ieon() (HPET_IE_ON) checks if (!devp->hd_ireqfreq) before enabling the timer interrupt, HPET_IRQFREQ neither rejects updates while HPET_IE is already active nor checks whether hpet_time_div(hpetp, arg) evaluates to 0. As a result, arming the timer with a valid frequency (for example, HPET_IRQFREQ with 1000 Hz followed by HPET_IE_ON) and then calling HPET_IRQFREQ with a large frequency (such as 0xffffffffUL) overwrites devp->hd_ireqfreq with 0 while the timer interrupt is active. When the next interrupt fires, hpet_interrupt() reads t = devp->hd_ireqfreq (0) and computes base = mc % t, crashing the kernel in hard-IRQ context:

  Oops: divide error: 0000 [#1] SMP KASAN PTI
  CPU: 0 UID: 0 PID: 0 Comm: swapper/0
  RIP: 0010:hpet_interrupt+0x20f/0x360
  Call Trace:
   <IRQ>
   __handle_irq_event_percpu+0x102/0x400
   handle_irq_event+0xa6/0x1c0
   handle_level_irq+0x205/0x5e0
   __common_interrupt+0x60/0x130
   common_interrupt+0x7a/0x90
   </IRQ>
  Kernel panic - not syncing: Fatal exception in interrupt

Reject HPET_IRQFREQ with -EBUSY when HPET_IE is set in devp->hd_flags, and return -EINVAL when hpet_time_div(hpetp, arg) evaluates to 0.

Tested in QEMU (-global hpet.hpet-intcap=0x0c24 with noapic) by opening /dev/hpet and calling ioctl(fd, HPET_IRQFREQ, 1000), ioctl(fd, HPET_IE_ON, 0), and ioctl(fd, HPET_IRQFREQ, 0xffffffffUL): on the unfixed kernel this immediately triggers the divide error panic in hpet_interrupt(), whereas on the fixed kernel HPET_IRQFREQ returns -EBUSY while HPET_IE is enabled and -EINVAL when hpet_time_div(hpetp, arg) is 0.

Fixes ba3f213f8a31 ("[PATCH] HPET: make frequency calculations 32 bit safe")
Fixes 273ef9509b79 ("drivers/char/hpet.c: fix periodic-emulation for delayed interrupts")
Assisted-by LLM

---
Changes in v2:
- Add Cc: [email protected] and include the QEMU test procedure and oops trace in the commit description per Greg Kroah-Hartman.
- Add Fixes: 273ef9509b79 ("drivers/char/hpet.c: fix periodic-emulation for delayed interrupts") for the mc % t division in hpet_interrupt().

Quoted 3 lines
 drivers/char/hpet.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

You sent multiple independent patches, to multiple independent subsystems. The amount of these patches clearly suggest this was AI generated and most likely not tested.

More importantly, you sent all this work without properly organizing relevant patches into patchsets. This makes reviewing difficult and might cause multiple reviewers to address the same issue. Replying to the entire set is impossible and requires handling each patch independently, instead of applying or discarding the set. Maintainers also won't see the bigger picture of your work. Quite worrying.

This is on the verge of hostile patch: bomb us with so many contributions, we won't be able to handle them in efficient manner, like responding ONCE to ask you to slow down. Considering all this is untested and LLM generated, I have even more doubts whether this should be considered for review.

Please read kernel documentation BEFORE posting more work. It will explain you how to identify subsystems, how to organize your work per subsystem, how to document usage of LLM and how what you should not do if this was posted in a good faith.

Best regards,
Krzysztof