* [PATCH v3] usb: gadget: f_fs: don't free buffer prematurely
@ 2019-03-21 21:31 fei.yang
2019-03-22 5:36 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: fei.yang @ 2019-03-21 21:31 UTC (permalink / raw)
To: balbi, gregkh, andrzej.p, plr.vincent, fei.yang, jingx.shen,
john.stultz, linux-usb, linux-kernel
From: Fei Yang <fei.yang@intel.com>
The following kernel panic happens due to the io_data buffer gets deallocated
before the async io is completed. Add a check for the case where io_data buffer
should be deallocated by ffs_user_copy_worker.
[ 41.663334] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
[ 41.672099] #PF error: [normal kernel read fault]
[ 41.677356] PGD 20c974067 P4D 20c974067 PUD 20c973067 PMD 0
[ 41.683687] Oops: 0000 [#1] PREEMPT SMP
[ 41.687976] CPU: 1 PID: 7 Comm: kworker/u8:0 Tainted: G U 5.0.0-quilt-2e5dc0ac-00790-gd8c79f2-dirty #2
[ 41.705309] Workqueue: adb ffs_user_copy_worker
[ 41.705316] RIP: 0010:__vunmap+0x2a/0xc0
[ 41.705318] Code: 0f 1f 44 00 00 48 85 ff 0f 84 87 00 00 00 55 f7 c7 ff 0f 00 00 48 89 e5 41 55 41 89 f5 41 54 53 48 89 fb 75 71 e8 56 d7 ff ff <4c> 8b 60 48 4d 85 e4 74 76 48 89 df e8 25 ff ff ff 45 85 ed 74 46
[ 41.705320] RSP: 0018:ffffbc3a40053df0 EFLAGS: 00010286
[ 41.705322] RAX: 0000000000000000 RBX: ffffbc3a406f1000 RCX: 0000000000000000
[ 41.705323] RDX: 0000000000000001 RSI: 0000000000000001 RDI: 00000000ffffffff
[ 41.705324] RBP: ffffbc3a40053e08 R08: 000000000001fb79 R09: 0000000000000037
[ 41.705325] R10: ffffbc3a40053b68 R11: ffffbc3a40053cad R12: fffffffffffffff2
[ 41.705326] R13: 0000000000000001 R14: 0000000000000000 R15: ffffffffffffffff
[ 41.705328] FS: 0000000000000000(0000) GS:ffff9e2977a80000(0000) knlGS:0000000000000000
[ 41.705329] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 41.705330] CR2: 0000000000000048 CR3: 000000020c994000 CR4: 00000000003406e0
[ 41.705331] Call Trace:
[ 41.705338] vfree+0x50/0xb0
[ 41.705341] ffs_user_copy_worker+0xe9/0x1c0
[ 41.705344] process_one_work+0x19f/0x3e0
[ 41.705348] worker_thread+0x3f/0x3b0
[ 41.829766] kthread+0x12b/0x150
[ 41.833371] ? process_one_work+0x3e0/0x3e0
[ 41.838045] ? kthread_create_worker_on_cpu+0x70/0x70
[ 41.843695] ret_from_fork+0x3a/0x50
[ 41.847689] Modules linked in: hci_uart bluetooth ecdh_generic rfkill_gpio dwc3_pci dwc3 snd_usb_audio mei_me tpm_crb snd_usbmidi_lib xhci_pci xhci_hcd mei tpm snd_hwdep cfg80211 snd_soc_skl snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp snd_hda_ext_core snd_hda_core videobuf2_dma_sg crlmodule
[ 41.876880] CR2: 0000000000000048
[ 41.880584] ---[ end trace 2bc4addff0f2e673 ]---
[ 41.891346] RIP: 0010:__vunmap+0x2a/0xc0
[ 41.895734] Code: 0f 1f 44 00 00 48 85 ff 0f 84 87 00 00 00 55 f7 c7 ff 0f 00 00 48 89 e5 41 55 41 89 f5 41 54 53 48 89 fb 75 71 e8 56 d7 ff ff <4c> 8b 60 48 4d 85 e4 74 76 48 89 df e8 25 ff ff ff 45 85 ed 74 46
[ 41.916740] RSP: 0018:ffffbc3a40053df0 EFLAGS: 00010286
[ 41.922583] RAX: 0000000000000000 RBX: ffffbc3a406f1000 RCX: 0000000000000000
[ 41.930563] RDX: 0000000000000001 RSI: 0000000000000001 RDI: 00000000ffffffff
[ 41.938540] RBP: ffffbc3a40053e08 R08: 000000000001fb79 R09: 0000000000000037
[ 41.946520] R10: ffffbc3a40053b68 R11: ffffbc3a40053cad R12: fffffffffffffff2
[ 41.954502] R13: 0000000000000001 R14: 0000000000000000 R15: ffffffffffffffff
[ 41.962482] FS: 0000000000000000(0000) GS:ffff9e2977a80000(0000) knlGS:0000000000000000
[ 41.971536] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 41.977960] CR2: 0000000000000048 CR3: 000000020c994000 CR4: 00000000003406e0
[ 41.985930] Kernel panic - not syncing: Fatal exception
[ 41.991817] Kernel Offset: 0x16000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[ 42.009525] Rebooting in 10 seconds..
[ 52.014376] ACPI MEMORY or I/O RESET_REG.
Fixes: 772a7a724f6 ("usb: gadget: f_fs: Allow scatter-gather buffers")
Signed-off-by: Fei Yang <fei.yang@intel.com>
Reviewed-by: Manu Gautam <mgautam@codeaurora.org>
Tested-by: John Stultz <john.stultz@linaro.org>
---
v2: add tag: "Fixes: 772a7a724f6 ......", Reviewed-by and Tested-by.
v3: check data for NULL instead of "ret == -EIOCBQUEUED", which would be safer
and keep the original logic intact.
---
drivers/usb/gadget/function/f_fs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 20413c2..0bce731 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1133,7 +1133,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
error_mutex:
mutex_unlock(&epfile->mutex);
error:
- ffs_free_buffer(io_data);
+ if (data) /* don't free if there is iocb queued */
+ ffs_free_buffer(io_data);
return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] usb: gadget: f_fs: don't free buffer prematurely
2019-03-21 21:31 [PATCH v3] usb: gadget: f_fs: don't free buffer prematurely fei.yang
@ 2019-03-22 5:36 ` Greg KH
2019-03-22 6:12 ` Yang, Fei
0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2019-03-22 5:36 UTC (permalink / raw)
To: fei.yang
Cc: balbi, andrzej.p, plr.vincent, jingx.shen, john.stultz,
linux-usb, linux-kernel
On Thu, Mar 21, 2019 at 02:31:14PM -0700, fei.yang@intel.com wrote:
> From: Fei Yang <fei.yang@intel.com>
>
> The following kernel panic happens due to the io_data buffer gets deallocated
> before the async io is completed. Add a check for the case where io_data buffer
> should be deallocated by ffs_user_copy_worker.
>
> [ 41.663334] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
> [ 41.672099] #PF error: [normal kernel read fault]
> [ 41.677356] PGD 20c974067 P4D 20c974067 PUD 20c973067 PMD 0
> [ 41.683687] Oops: 0000 [#1] PREEMPT SMP
> [ 41.687976] CPU: 1 PID: 7 Comm: kworker/u8:0 Tainted: G U 5.0.0-quilt-2e5dc0ac-00790-gd8c79f2-dirty #2
> [ 41.705309] Workqueue: adb ffs_user_copy_worker
> [ 41.705316] RIP: 0010:__vunmap+0x2a/0xc0
> [ 41.705318] Code: 0f 1f 44 00 00 48 85 ff 0f 84 87 00 00 00 55 f7 c7 ff 0f 00 00 48 89 e5 41 55 41 89 f5 41 54 53 48 89 fb 75 71 e8 56 d7 ff ff <4c> 8b 60 48 4d 85 e4 74 76 48 89 df e8 25 ff ff ff 45 85 ed 74 46
> [ 41.705320] RSP: 0018:ffffbc3a40053df0 EFLAGS: 00010286
> [ 41.705322] RAX: 0000000000000000 RBX: ffffbc3a406f1000 RCX: 0000000000000000
> [ 41.705323] RDX: 0000000000000001 RSI: 0000000000000001 RDI: 00000000ffffffff
> [ 41.705324] RBP: ffffbc3a40053e08 R08: 000000000001fb79 R09: 0000000000000037
> [ 41.705325] R10: ffffbc3a40053b68 R11: ffffbc3a40053cad R12: fffffffffffffff2
> [ 41.705326] R13: 0000000000000001 R14: 0000000000000000 R15: ffffffffffffffff
> [ 41.705328] FS: 0000000000000000(0000) GS:ffff9e2977a80000(0000) knlGS:0000000000000000
> [ 41.705329] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 41.705330] CR2: 0000000000000048 CR3: 000000020c994000 CR4: 00000000003406e0
> [ 41.705331] Call Trace:
> [ 41.705338] vfree+0x50/0xb0
> [ 41.705341] ffs_user_copy_worker+0xe9/0x1c0
> [ 41.705344] process_one_work+0x19f/0x3e0
> [ 41.705348] worker_thread+0x3f/0x3b0
> [ 41.829766] kthread+0x12b/0x150
> [ 41.833371] ? process_one_work+0x3e0/0x3e0
> [ 41.838045] ? kthread_create_worker_on_cpu+0x70/0x70
> [ 41.843695] ret_from_fork+0x3a/0x50
> [ 41.847689] Modules linked in: hci_uart bluetooth ecdh_generic rfkill_gpio dwc3_pci dwc3 snd_usb_audio mei_me tpm_crb snd_usbmidi_lib xhci_pci xhci_hcd mei tpm snd_hwdep cfg80211 snd_soc_skl snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp snd_hda_ext_core snd_hda_core videobuf2_dma_sg crlmodule
> [ 41.876880] CR2: 0000000000000048
> [ 41.880584] ---[ end trace 2bc4addff0f2e673 ]---
> [ 41.891346] RIP: 0010:__vunmap+0x2a/0xc0
> [ 41.895734] Code: 0f 1f 44 00 00 48 85 ff 0f 84 87 00 00 00 55 f7 c7 ff 0f 00 00 48 89 e5 41 55 41 89 f5 41 54 53 48 89 fb 75 71 e8 56 d7 ff ff <4c> 8b 60 48 4d 85 e4 74 76 48 89 df e8 25 ff ff ff 45 85 ed 74 46
> [ 41.916740] RSP: 0018:ffffbc3a40053df0 EFLAGS: 00010286
> [ 41.922583] RAX: 0000000000000000 RBX: ffffbc3a406f1000 RCX: 0000000000000000
> [ 41.930563] RDX: 0000000000000001 RSI: 0000000000000001 RDI: 00000000ffffffff
> [ 41.938540] RBP: ffffbc3a40053e08 R08: 000000000001fb79 R09: 0000000000000037
> [ 41.946520] R10: ffffbc3a40053b68 R11: ffffbc3a40053cad R12: fffffffffffffff2
> [ 41.954502] R13: 0000000000000001 R14: 0000000000000000 R15: ffffffffffffffff
> [ 41.962482] FS: 0000000000000000(0000) GS:ffff9e2977a80000(0000) knlGS:0000000000000000
> [ 41.971536] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 41.977960] CR2: 0000000000000048 CR3: 000000020c994000 CR4: 00000000003406e0
> [ 41.985930] Kernel panic - not syncing: Fatal exception
> [ 41.991817] Kernel Offset: 0x16000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> [ 42.009525] Rebooting in 10 seconds..
> [ 52.014376] ACPI MEMORY or I/O RESET_REG.
>
> Fixes: 772a7a724f6 ("usb: gadget: f_fs: Allow scatter-gather buffers")
> Signed-off-by: Fei Yang <fei.yang@intel.com>
> Reviewed-by: Manu Gautam <mgautam@codeaurora.org>
> Tested-by: John Stultz <john.stultz@linaro.org>
> ---
> v2: add tag: "Fixes: 772a7a724f6 ......", Reviewed-by and Tested-by.
> v3: check data for NULL instead of "ret == -EIOCBQUEUED", which would be safer
> and keep the original logic intact.
If it "fixes" a problem in 5.0, we should add a:
Cc: stable <stable@vger.kernel.org>
to it as well.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH v3] usb: gadget: f_fs: don't free buffer prematurely
2019-03-22 5:36 ` Greg KH
@ 2019-03-22 6:12 ` Yang, Fei
0 siblings, 0 replies; 3+ messages in thread
From: Yang, Fei @ 2019-03-22 6:12 UTC (permalink / raw)
To: Greg KH
Cc: balbi, andrzej.p, plr.vincent, Shen, JingX, john.stultz,
linux-usb, linux-kernel
>> The following kernel panic happens due to the io_data buffer gets
>> deallocated before the async io is completed. Add a check for the case
>> where io_data buffer should be deallocated by ffs_user_copy_worker.
>>
>> [ 41.663334] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
>>
>> Fixes: 772a7a724f6 ("usb: gadget: f_fs: Allow scatter-gather buffers")
>> Signed-off-by: Fei Yang <fei.yang@intel.com>
>> Reviewed-by: Manu Gautam <mgautam@codeaurora.org>
>> Tested-by: John Stultz <john.stultz@linaro.org>
>> ---
>> v2: add tag: "Fixes: 772a7a724f6 ......", Reviewed-by and Tested-by.
>> v3: check data for NULL instead of "ret == -EIOCBQUEUED", which would be safer
>> and keep the original logic intact.
>
> If it "fixes" a problem in 5.0, we should add a:
> Cc: stable <stable@vger.kernel.org>
> to it as well.
Sent [PATCH v4] with Cc: stable
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-22 6:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-21 21:31 [PATCH v3] usb: gadget: f_fs: don't free buffer prematurely fei.yang
2019-03-22 5:36 ` Greg KH
2019-03-22 6:12 ` Yang, Fei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).