Re: [PATCH] Input: pegasus-notetaker - fix slab-out-of-bounds in pegasus_irq

[Martin Kepplinger-Novakovic <martink@posteo.de>]

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@vger.kernel.org
> Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.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@posteo.de>

thank you