Re: [PATCH v2 1/4] HID: sony: use dedicated raw_event() handlers in sony_raw_event()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Race Condition / Null Pointer Dereference in RB4_GUITAR_PS5 initialization.
- [High] Sleeping in Atomic Context in rb3_pro_instrument_raw_event.
--

commit ce2aa734b3cbe3b5ae8274fe7a1ef679e83470b5
Author: Rosalie Wanders <rosalie@mailbox.org>
Subject: HID: sony: use dedicated raw_event() handlers in sony_raw_event()

This commit simplifies how the sony driver processes raw events by introducing
dedicated handler function pointers inside the sony_sc structure. The
appropriate handler is now assigned during device configuration based on
hardware quirks.

> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 315343415e8f1..7da1fd1e52fea 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c

[ ... ]

> +static int rb3_pro_instrument_raw_event(struct sony_sc *sc, u8 *rd, int size)
> +{
> +	/* Rock Band 3 PS3 Pro instruments set rd[24] to 0xE0 when they're
> +	 * sending full reports, and 0x02 when only sending navigation.
> +	 */
> +	if (size < 25 || rd[24] != 0x02)
> +		return 0;
> +
> +	/* Only attempt to enable full report every 8 seconds */
> +	if (time_after(jiffies, sc->rb3_pro_poke_jiffies)) {
> +		sc->rb3_pro_poke_jiffies = jiffies + secs_to_jiffies(8);
> +		rb3_pro_instrument_enable_full_report(sc);

[Severity: High]
This is a pre-existing issue, but can this call sequence sleep in an atomic
context?

The raw event handler usually executes in an atomic context, such as a USB URB
completion handler. This path calls rb3_pro_instrument_enable_full_report()
which allocates memory using GFP_KERNEL and issues a synchronous USB control
transfer via hid_hw_raw_request().

Could calling these blocking functions in atomic context trigger a might_sleep
warning or lead to a kernel panic?

> +	}
> +
> +	return 0;
> +}

[ ... ]

> -static void rb4_ps5_guitar_parse_report(struct sony_sc *sc, u8 *rd, int size)
> +static int rb4_ps5_guitar_raw_event(struct sony_sc *sc, u8 *rd, int size)
>  {
>  	u8 charging_status;
>  	u8 battery_data;
> @@ -1094,6 +1150,9 @@ static void rb4_ps5_guitar_parse_report(struct sony_sc *sc, u8 *rd, int size)
>  	u8 battery_status;
>  	unsigned long flags;
>  
> +	if (unlikely(size != 64 || rd[0] != 0x01))
> +		return 0;
> +
>  	/*
>  	 * Rock Band 4 PS5 guitars have whammy and
>  	 * tilt functionality, they're located at

[Severity: High]
This isn't a bug introduced by this patch, but could this handler dereference
a null pointer if an event arrives during device initialization?

Looking at sony_input_configured(), the raw event handler for the guitar is
registered, and hid_hw_open() is called because the device has battery support.
However, sc->input_dev is not assigned until after these initialization steps.

If the device sends a report immediately after the hardware connection opens,
would this event handler trigger and attempt to call input_report_abs() using
a null sc->input_dev pointer?

> @@ -1138,65 +1197,22 @@ static void rb4_ps5_guitar_parse_report(struct sony_sc *sc, u8 *rd, int size)
>  	spin_unlock_irqrestore(&sc->lock, flags);
>  
>  	input_sync(sc->input_dev);
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616205042.6671-2-rosalie@mailbox.org?part=1