[0/3] HID: hid-bigbenff: fixing three crash bugs in a gamepad driver

Message

Hanno Zulla Feb. 17, 2020, 3:24 p.m. UTC

Hi there,

the hid-bigbenff.c had three bugs causing possible kernel crashes.

The first patch fixes a double free during device removal, which was 
caused by a wrong use of input_ff_create_memless(). The 
"driver-specific data to be passed into play_effect" parameter of 
input_ff_create_memless() would later be freed automatically when the ff 
device is removed. Since the driver also uses the managed resource API, 
it would automatically free the memory of this parameter twice, causing 
a general protection fault moments later.

The second patch fixes the error path after hid_hw_start(), as a call 
to hid_hw_stop() is required in case of an error.

The second patch also removes the hid_hw_close() call during device 
removal, as several other hid device drivers don't call this routine, 
either.

The third patch adds a flag to avoid a race condition when there is 
still scheduled work left (or newly being scheduled) during or after 
device removal, which could cause a kernel crash.

Thanks in advance for your review & kind regards,

Hanno

Comments

Benjamin Tissoires Feb. 18, 2020, 10:40 a.m. UTC | #1

Hi Hanno,

On Mon, Feb 17, 2020 at 4:24 PM Hanno Zulla <[e-mail]> wrote:
>
> Hi there,
>
> the hid-bigbenff.c had three bugs causing possible kernel crashes.
>
> The first patch fixes a double free during device removal, which was
> caused by a wrong use of input_ff_create_memless(). The
> "driver-specific data to be passed into play_effect" parameter of
> input_ff_create_memless() would later be freed automatically when the ff
> device is removed. Since the driver also uses the managed resource API,
> it would automatically free the memory of this parameter twice, causing
> a general protection fault moments later.
>
> The second patch fixes the error path after hid_hw_start(), as a call
> to hid_hw_stop() is required in case of an error.
>
> The second patch also removes the hid_hw_close() call during device
> removal, as several other hid device drivers don't call this routine,
> either.
>
> The third patch adds a flag to avoid a race condition when there is
> still scheduled work left (or newly being scheduled) during or after
> device removal, which could cause a kernel crash.
>
> Thanks in advance for your review & kind regards,
>

I think the patches are correct (have you tested them with actual HW?).
However, checkpatch complains that the From and Signed-off-by email
differ. Can you send a v2 with a fix for that?

Cheers,
Benjamin