Re: [PATCH v3] hid-sony: Prevent crash when rumble effects are still loaded at USB disconnect

[Cameron Gutman <aicommander@gmail.com>]

On 06/11/2016 05:00 AM, Manuel Reimer wrote:
> Hello,
> 
> I did some more testing. Now I added printk messages to start and end of ml_effect_timer and to hl_ff_destroy. Result:
> 
> [  513.493511] ml_effect_timer start
> [  513.746964] ml_effect_timer end
> [  515.107003] hid-sony: Sending to uninitialized device failed!
> [  515.333520] hid-sony: Sending to uninitialized device failed!
> [  515.415381] hid-sony: Sending to uninitialized device failed!
> [  520.476860] ml_effect_timer start
> [  520.677003] BUG: unable to handle kernel NULL pointer dereference at 00000000000000d8
> 
> The hid-sony messages are created by my last patch to fix the hid-sony driver. They show that some sending attempts have been cancelled, as the device is about to be destroyed.
> 
> Quite some time after that there in fact is another attempt to call ml_effect_timer, so the timer still was active. Tomorrow I'll add additional printk lines to the hid-sony destroy function to see if this finished executing before this unwanted timer call arrives.
> 
> This also shows that ml_ff_destroy is not the right place to cancel the timer. ml_ff_destroy is called as soon as I exit fftest. It is not called at all on USB disconnect.
> 
> I now guess this can also be reproduced with the xpad driver, but it requires some fiddling with fftest. It took me ten minutes this time to get the bug triggered. I think the way to trigger the bug is to start effect 5 and shortly after that effect 4. With some luck the USB plug is pulled before event 4 is actually started.
> 

Can you try applying the following patch on a clean source tree and see if it resolves your issue?

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index fcc6c33..6366e9a 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -501,6 +501,7 @@ static void ml_ff_destroy(struct ff_device *ff)
 {
        struct ml_device *ml = ff->private;
 
+       del_timer_sync(&ml->timer);
        kfree(ml->private);
 }