[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6114c4ed1f0145ca6deb96bccd0ebde145fcf68f.camel@posteo.de>
Date: Wed, 09 Apr 2025 14:30:49 +0000
From: Martin Kepplinger-Novakovic <martink@...teo.de>
To: Vasiliy Kovalev <kovalev@...linux.org>, Dmitry Torokhov
<dmitry.torokhov@...il.com>, Javier Carrasco
<javier.carrasco.cruz@...il.com>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: lvc-project@...uxtesting.org
Subject: Re: [PATCH] Input: pegasus-notetaker - fix slab-out-of-bounds in
pegasus_irq
Am Mittwoch, dem 02.04.2025 um 18:47 +0300 schrieb Vasiliy Kovalev:
> Fix a slab-out-of-bounds error in pegasus_irq reported by KASAN,
> caused by insufficient packet size validation. The driver relied
> on usb_maxpacket() to set the data length, which could be smaller
> than the 6 bytes expected by pegasus_parse_packet(), leading to
> oob reads at le16_to_cpup() calls when accessing coordinates.
>
> Introduce NOTETAKER_PKGLEN_SIZE define with a value of 6 bytes,
> reflecting the typical interrupt IN endpoint packet format for this
> device family. Add a check in pegasus_probe() to ensure the data
> length is at least this size, logging an info message if
> usb_maxpacket()
> returns a smaller value and adjusting it accordingly. This prevents
> buffer under-allocation while accommodating devices that might send
> shorter packets, as various brandings (e.g., Pegasus Mobile
> Notetaker,
> IRISnotes Express) may differ in implementation.
>
> KASAN report:
> BUG: KASAN: slab-out-of-bounds in pegasus_irq (little_endian.h:67
> pegasus_notetaker.c:153 pegasus_notetaker.c:183)
> Read of size 2 at addr ffff888009a01da2 by task (udev-worker)/985
> CPU: 0 PID: 985 Comm: (udev-worker) Tainted: G OE 6.14.0-un-def-
> alt0.rc6.kasan #1
> Hardware: QEMU Standard PC (Q35 + ICH9, 2009)
> Call Trace:
> <IRQ>
> dump_stack_lvl (lib/dump_stack.c:122)
> print_report (mm/kasan/report.c:521)
> kasan_report (mm/kasan/report.c:636)
> pegasus_irq (little_endian.h:67 pegasus_notetaker.c:153
> pegasus_notetaker.c:183)
> __usb_hcd_giveback_urb (drivers/usb/core/hcd.c:1650)
> usb_hcd_giveback_urb (drivers/usb/core/hcd.c:1735)
> dummy_timer (drivers/usb/gadget/udc/dummy_hcd.c:1995)
> __hrtimer_run_queues (kernel/time/hrtimer.c:1865)
> hrtimer_run_softirq (kernel/time/hrtimer.c:1884)
> handle_softirqs (kernel/softirq.c:561)
> __irq_exit_rcu (kernel/softirq.c:662)
> irq_exit_rcu (kernel/softirq.c:680)
> sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1049)
> </IRQ>
>
> Found by Linux Verification Center (linuxtesting.org) with
> "USB Gadget Tests"[1]:
>
> $ make input-tab-pegasus
> $ sudo ./src/input-tab-pegasus/input-tab-pegasus --invalid_ep_int_len
>
> [1] Link: https://github.com/kovalev0/usb-gadget-tests
> Fixes: 1afca2b66aac ("Input: add Pegasus Notetaker tablet driver")
> Cc: stable@...r.kernel.org
> Signed-off-by: Vasiliy Kovalev <kovalev@...linux.org>
> ---
> drivers/input/tablet/pegasus_notetaker.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/input/tablet/pegasus_notetaker.c
> b/drivers/input/tablet/pegasus_notetaker.c
> index 8d6b71d5979316..e578720585c2c8 100644
> --- a/drivers/input/tablet/pegasus_notetaker.c
> +++ b/drivers/input/tablet/pegasus_notetaker.c
> @@ -70,6 +70,15 @@
> #define PEN_BUTTON_PRESSED BIT(1)
> #define PEN_TIP BIT(0)
>
> +/* Minimum packet size for interrupt IN endpoint.
> + * Packet format (expected 6 bytes, though some devices may send
> less):
> + * - Byte 0: Packet type (command or status)
> + * - Byte 1: Button/tip state
> + * - Bytes 2-3: X coordinate (16-bit, little-endian)
> + * - Bytes 4-5: Y coordinate (16-bit, little-endian)
> + */
> +#define NOTETAKER_PKGLEN_SIZE 6
> +
> struct pegasus {
> unsigned char *data;
> u8 data_len;
> @@ -311,6 +320,16 @@ static int pegasus_probe(struct usb_interface
> *intf,
> }
>
> pegasus->data_len = usb_maxpacket(dev, pipe);
> + /* Ensure buffer is at least NOTETAKER_PKGLEN_SIZE to avoid
> oob
> + * access in pegasus_parse_packet(). Adjust if endpoint
> reports a
> + * smaller size, as some devices might send shorter packets.
> + */
> + if (pegasus->data_len < NOTETAKER_PKGLEN_SIZE) {
> + dev_info(&intf->dev,
> + "Int in endpoint data_len adjusted from %d
> to minimum %d\n",
> + pegasus->data_len, NOTETAKER_PKGLEN_SIZE);
> + pegasus->data_len = NOTETAKER_PKGLEN_SIZE;
> + }
>
> pegasus->data = usb_alloc_coherent(dev, pegasus->data_len,
> GFP_KERNEL,
> &pegasus->data_dma);
Tested-by: Martin Kepplinger-Novakovic <martink@...teo.de>
thank you
Powered by blists - more mailing lists