Re: [PATCH v2] HID: wacom: fix slab-out-of-bounds write in wacom_wac_queue_insert

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Thu, May 28, 2026 at 06:42:29PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] wacom_wac_queue_flush() allocates memory with GFP_KERNEL while executing in an atomic context, leading to a 'scheduling while atomic' panic.
> --
> 
> commit 70e9615b93de485ba1816f95e27e25b054df4b6a
> Author: Jinmo Yang <jinmo44.yang@gmail.com>
> 
> HID: wacom: fix slab-out-of-bounds write in wacom_wac_queue_insert
> 
> This commit fixes a slab-out-of-bounds write in wacom_wac_queue_insert().
> It prevents kfifo_skip() from operating on an empty fifo and validates
> the return value of kfifo_in().
> 
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index a32320b351e3e..489ca68f1560f 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> 
> [ ... ]
> 
> > @@ -62,7 +62,9 @@ static void wacom_wac_queue_insert(struct hid_device *hdev,
> >  		kfifo_skip(fifo);
> >  	}
> >  
> > -	kfifo_in(fifo, raw_data, size);
> > +	if (!kfifo_in(fifo, raw_data, size))
> > +		hid_warn_ratelimited(hdev, "%s: report is too large (%d)\n",
> > +				     __func__, size);
> >  }
> >  
> >  static void wacom_wac_queue_flush(struct hid_device *hdev,
> >  				  struct kfifo_rec_ptr_2 *fifo)
> >  {
> >  	while (!kfifo_is_empty(fifo)) {
> >  		int size = kfifo_peek_len(fifo);
> >  		u8 *buf;
> >  		unsigned int count;
> >  		int err;
> >  
> >  		buf = kzalloc(size, GFP_KERNEL);
> 
> [Severity: High]
> This is a pre-existing issue, but is it safe to use GFP_KERNEL here?
> 
> Looking at the call chain, wacom_wac_queue_flush() is invoked via
> wacom_wac_pen_serial_enforce() from the driver's .raw_event callback,
> wacom_raw_event().
> 
> For USB HID devices, this callback is typically triggered by hid_irq_in(),
> which is an URB completion handler that runs in an atomic context.
> 
> Since using GFP_KERNEL allows the allocator to sleep, could this lead to a
> scheduling while atomic panic? Would it be better to use GFP_ATOMIC instead?
> 

This looks like valid analysis. Would you be willing to send a
[completely separate] patch?

Thanks.

-- 
Dmitry