Skip to content

Commit 875115b

Browse files
pip-izonydtor
authored andcommitted
Input: ims-pcu - fix heap-buffer-overflow in ims_pcu_process_data()
The `ims_pcu_process_data()` processes incoming URB data byte by byte. However, it fails to check if the `read_pos` index exceeds IMS_PCU_BUF_SIZE. If a malicious USB device sends a packet larger than IMS_PCU_BUF_SIZE, `read_pos` will increment indefinitely. Moreover, since `read_pos` is located immediately after `read_buf`, the attacker can overwrite `read_pos` itself to arbitrarily control the index. This manipulated `read_pos` is subsequently used in `ims_pcu_handle_response()` to copy data into `cmd_buf`, leading to a heap buffer overflow. Specifically, an attacker can overwrite the `cmd_done.wait.head` located at offset 136 relative to `cmd_buf` in the `ims_pcu_handle_response()`. Consequently, when the driver calls `complete(&pcu->cmd_done)`, it triggers a control flow hijack by using the manipulated pointer. Fix this by adding a bounds check for `read_pos` before writing to `read_buf`. If the packet is too long, discard it, log a warning, and reset the parser state. Fixes: 628329d ("Input: add IMS Passenger Control Unit driver") Co-developed-by: Sanghoon Choi <[email protected]> Signed-off-by: Sanghoon Choi <[email protected]> Signed-off-by: Seungjin Bae <[email protected]> Link: https://patch.msgid.link/[email protected] [dtor: factor out resetting packet state, reset checksum as well] Signed-off-by: Dmitry Torokhov <[email protected]>
1 parent f7a78e8 commit 875115b

1 file changed

Lines changed: 26 additions & 6 deletions

File tree

drivers/input/misc/ims-pcu.c

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,14 @@ static void ims_pcu_handle_response(struct ims_pcu *pcu)
438438
}
439439
}
440440

441+
static void ims_pcu_reset_packet(struct ims_pcu *pcu)
442+
{
443+
pcu->have_stx = true;
444+
pcu->have_dle = false;
445+
pcu->read_pos = 0;
446+
pcu->check_sum = 0;
447+
}
448+
441449
static void ims_pcu_process_data(struct ims_pcu *pcu, struct urb *urb)
442450
{
443451
int i;
@@ -450,6 +458,14 @@ static void ims_pcu_process_data(struct ims_pcu *pcu, struct urb *urb)
450458
continue;
451459

452460
if (pcu->have_dle) {
461+
if (pcu->read_pos >= IMS_PCU_BUF_SIZE) {
462+
dev_warn(pcu->dev,
463+
"Packet too long (%d bytes), discarding\n",
464+
pcu->read_pos);
465+
ims_pcu_reset_packet(pcu);
466+
continue;
467+
}
468+
453469
pcu->have_dle = false;
454470
pcu->read_buf[pcu->read_pos++] = data;
455471
pcu->check_sum += data;
@@ -462,10 +478,8 @@ static void ims_pcu_process_data(struct ims_pcu *pcu, struct urb *urb)
462478
dev_warn(pcu->dev,
463479
"Unexpected STX at byte %d, discarding old data\n",
464480
pcu->read_pos);
481+
ims_pcu_reset_packet(pcu);
465482
pcu->have_stx = true;
466-
pcu->have_dle = false;
467-
pcu->read_pos = 0;
468-
pcu->check_sum = 0;
469483
break;
470484

471485
case IMS_PCU_PROTOCOL_DLE:
@@ -485,12 +499,18 @@ static void ims_pcu_process_data(struct ims_pcu *pcu, struct urb *urb)
485499
ims_pcu_handle_response(pcu);
486500
}
487501

488-
pcu->have_stx = false;
489-
pcu->have_dle = false;
490-
pcu->read_pos = 0;
502+
ims_pcu_reset_packet(pcu);
491503
break;
492504

493505
default:
506+
if (pcu->read_pos >= IMS_PCU_BUF_SIZE) {
507+
dev_warn(pcu->dev,
508+
"Packet too long (%d bytes), discarding\n",
509+
pcu->read_pos);
510+
ims_pcu_reset_packet(pcu);
511+
continue;
512+
}
513+
494514
pcu->read_buf[pcu->read_pos++] = data;
495515
pcu->check_sum += data;
496516
break;

0 commit comments

Comments
 (0)