Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753834AbcLHPSe (ORCPT ); Thu, 8 Dec 2016 10:18:34 -0500 Received: from mail-wm0-f50.google.com ([74.125.82.50]:35718 "EHLO mail-wm0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752087AbcLHPSb (ORCPT ); Thu, 8 Dec 2016 10:18:31 -0500 From: Michal Nazarewicz To: Baolin Wang , balbi@kernel.org Cc: gregkh@linuxfoundation.org, felixhaedicke@web.de, jilin@nvidia.com, dan.carpenter@oracle.com, lars@metafoo.de, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, broonie@kernel.org, baolin.wang@linaro.org Subject: Re: [PATCH] usb: gadget: f_fs: Fix possibe deadlock In-Reply-To: <470947805c9f50b9649bea4f9814b2dcb6ebcc45.1481197710.git.baolin.wang@linaro.org> Organization: http://mina86.com/ References: <470947805c9f50b9649bea4f9814b2dcb6ebcc45.1481197710.git.baolin.wang@linaro.org> User-Agent: Notmuch/0.19+53~g2e63a09 (http://notmuchmail.org) Emacs/26.0.50.2 (x86_64-unknown-linux-gnu) Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAJFBMVEWbfGlUPDDHgE57V0jUupKjgIObY0PLrom9mH4dFRK4gmjPs41MxjOgAAACP0lEQVQ4T23Sv2vbQBQHcBk1xE6WyALX107VUEgmn6+ouUwpEQQ6uRjttkWP4CkBg2M0BQLBdPFZYPsyFYo7qEtKDQ7on+t7+nF2Ux8ahD587717OmNYrOvycHsZ+o2r051wHTHysAvGb8ygvgu4QWT0sCmkgZCIEnlV2X8BtyraazFGDuxhmKSQJMlwHQ7v5MHSNxmz78rfElwAa3ieVD9e+hBhjaPDDG6NgFo2f4wBMNIo5YmRtF0RyDgFjJjlMIWbnuM4x9MMfABGTlN4qgIQB4A1DEyA1BHWtfeWNUMwiVJKoqh97KrkOO+qzgluVYLvFCUKAX73nONeBr7BGMdM6Sg0kuep03VywLaIzRiVr+GAzKlpQIsAFnWAG2e6DT5WmWDiudZMIc6hYrMOmeMQK9WX0B+/RfjzL9DI7Y9/Iayn29Ci0r2i4f9gMimMSZLCDMalgQGU5hnUtqAN0OGvEmO1Wnl0C0wWSCEHnuHBqmygxdxA8oWXwbipoc1EoNR9DqOpBpOJrnr0criQab9ZT4LL+wI+K7GBQH30CrhUruilgP9DRTrhVWZCiAyILP+wiuLeCKGTD6r/nc8LOJcAwR6IBTUs+7CASw3QFZ0MdA2PI3zNziH4ZKVhXCRMBjeZ1DWMekKwDCASwExy+NQ86TaykaDAFHO4aP48y4fIcDM5yOG8GcTLbOyp8A8azjJI93JFd1EA6yN8sSxMQJWoABqniRZVykYgRXErzrdqExAoUrRb0xfRp8p2A/4XmfilTtkDZ4cAAAAASUVORK5CYII= X-Face: -TR8(rDTHy/(xl?SfWd1|3:TTgDIatE^t'vop%*gVg[kn$t{EpK(P"VQ=~T2#ysNmJKN$"yTRLB4YQs$4{[.]Fc1)*O]3+XO^oXM>Q#b^ix,O)Zbn)q[y06$`e3?C)`CwR9y5riE=fv^X@x$y?D:XO6L&x4f-}}I4=VRNwiA^t1-ZrVK^07.Pi/57c_du'& X-PGP: 50751FF4 X-PGP-FP: AC1F 5F5C D418 88F8 CC84 5858 2060 4012 5075 1FF4 X-Hashcash: 1:20:161208:dan.carpenter@oracle.com::9rPOqYXYwAVV/i95:00000000000000000000000000000000000000pVy X-Hashcash: 1:20:161208:gregkh@linuxfoundation.org::ulJjeIYu0wlure7S:000000000000000000000000000000000000sZE X-Hashcash: 1:20:161208:baolin.wang@linaro.org::jqkTvq8N7IjHS7NT:0000000000000000000000000000000000000001Iz/ X-Hashcash: 1:20:161208:broonie@kernel.org::Rj/pCdmKSXiNZDeL:00000000000000000000000000000000000000000002o74 X-Hashcash: 1:20:161208:linux-kernel@vger.kernel.org::XNoRyYXjdBR7Cx3T:000000000000000000000000000000000EJnm X-Hashcash: 1:20:161208:linux-usb@vger.kernel.org::tyHciHh6eUr8Ymng:000000000000000000000000000000000000Ci5R X-Hashcash: 1:20:161208:lars@metafoo.de::kHkF+3nc3+iIOCZd:002eSk X-Hashcash: 1:20:161208:jilin@nvidia.com::gUU8+5D4w7/yRprV:05d6k X-Hashcash: 1:20:161208:felixhaedicke@web.de::k04pLKkEFQ7V37th:000000000000000000000000000000000000000002sK/ X-Hashcash: 1:20:161208:balbi@kernel.org::mQ75GIpDiXC9Zcql:0B4l6 X-Hashcash: 1:20:161208:baolin.wang@linaro.org::nGDvnSg0tjmRmZ+E:0000000000000000000000000000000000000009Q9w Date: Thu, 08 Dec 2016 16:18:26 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uB8FIidd019281 Content-Length: 7079 Lines: 138 On Thu, Dec 08 2016, Baolin Wang wrote: > When system try to close /dev/usb-ffs/adb/ep0 on one core, at the same > time another core try to attach new UDC, which will cause deadlock as > below scenario. Thus we should release ffs lock before issuing > unregister_gadget_item(). > > [ 52.642225] c1 ====================================================== > [ 52.642228] c1 [ INFO: possible circular locking dependency detected ] > [ 52.642236] c1 4.4.6+ #1 Tainted: G W O > [ 52.642241] c1 ------------------------------------------------------- > [ 52.642245] c1 usb ffs open/2808 is trying to acquire lock: > [ 52.642270] c0 (udc_lock){+.+.+.}, at: [] > usb_gadget_unregister_driver+0x3c/0xc8 > [ 52.642272] c1 but task is already holding lock: > [ 52.642283] c0 (ffs_lock){+.+.+.}, at: [] > ffs_data_clear+0x30/0x140 > [ 52.642285] c1 which lock already depends on the new lock. > [ 52.642287] c1 > the existing dependency chain (in reverse order) is: > [ 52.642295] c0 > -> #1 (ffs_lock){+.+.+.}: > [ 52.642307] c0 [] __lock_acquire+0x20f0/0x2238 > [ 52.642314] c0 [] lock_acquire+0xe4/0x298 > [ 52.642322] c0 [] mutex_lock_nested+0x7c/0x3cc > [ 52.642328] c0 [] ffs_func_bind+0x504/0x6e8 > [ 52.642334] c0 [] usb_add_function+0x84/0x184 > [ 52.642340] c0 [] configfs_composite_bind+0x264/0x39c > [ 52.642346] c0 [] udc_bind_to_driver+0x58/0x11c > [ 52.642352] c0 [] usb_udc_attach_driver+0x90/0xc8 > [ 52.642358] c0 [] gadget_dev_desc_UDC_store+0xd4/0x128 > [ 52.642369] c0 [] configfs_write_file+0xd0/0x13c > [ 52.642376] c0 [] vfs_write+0xb8/0x214 > [ 52.642381] c0 [] SyS_write+0x54/0xb0 > [ 52.642388] c0 [] el0_svc_naked+0x24/0x28 > [ 52.642395] c0 > -> #0 (udc_lock){+.+.+.}: > [ 52.642401] c0 [] print_circular_bug+0x84/0x2e4 > [ 52.642407] c0 [] __lock_acquire+0x2138/0x2238 > [ 52.642412] c0 [] lock_acquire+0xe4/0x298 > [ 52.642420] c0 [] mutex_lock_nested+0x7c/0x3cc > [ 52.642427] c0 [] usb_gadget_unregister_driver+0x3c/0xc8 > [ 52.642432] c0 [] unregister_gadget_item+0x28/0x44 > [ 52.642439] c0 [] ffs_data_clear+0x138/0x140 > [ 52.642444] c0 [] ffs_data_reset+0x20/0x6c > [ 52.642450] c0 [] ffs_data_closed+0xac/0x12c > [ 52.642454] c0 [] ffs_ep0_release+0x20/0x2c > [ 52.642460] c0 [] __fput+0xb0/0x1f4 > [ 52.642466] c0 [] ____fput+0x20/0x2c > [ 52.642473] c0 [] task_work_run+0xb4/0xe8 > [ 52.642482] c0 [] do_exit+0x360/0xb9c > [ 52.642487] c0 [] do_group_exit+0x4c/0xb0 > [ 52.642494] c0 [] get_signal+0x380/0x89c > [ 52.642501] c0 [] do_signal+0x154/0x518 > [ 52.642507] c0 [] do_notify_resume+0x70/0x78 > [ 52.642512] c0 [] work_pending+0x1c/0x20 > [ 52.642514] c1 > other info that might help us debug this: > [ 52.642517] c1 Possible unsafe locking scenario: > [ 52.642518] c1 CPU0 CPU1 > [ 52.642520] c1 ---- ---- > [ 52.642525] c0 lock(ffs_lock); > [ 52.642529] c0 lock(udc_lock); > [ 52.642533] c0 lock(ffs_lock); > [ 52.642537] c0 lock(udc_lock); > [ 52.642539] c1 > *** DEADLOCK *** > [ 52.642543] c1 1 lock held by usb ffs open/2808: > [ 52.642555] c0 #0: (ffs_lock){+.+.+.}, at: [] > ffs_data_clear+0x30/0x140 > [ 52.642557] c1 stack backtrace: > [ 52.642563] c1 CPU: 1 PID: 2808 Comm: usb ffs open Tainted: G > [ 52.642565] c1 Hardware name: Spreadtrum SP9860g Board (DT) > [ 52.642568] c1 Call trace: > [ 52.642573] c1 [] dump_backtrace+0x0/0x170 > [ 52.642577] c1 [] show_stack+0x20/0x28 > [ 52.642583] c1 [] dump_stack+0xa8/0xe0 > [ 52.642587] c1 [] print_circular_bug+0x1fc/0x2e4 > [ 52.642591] c1 [] __lock_acquire+0x2138/0x2238 > [ 52.642595] c1 [] lock_acquire+0xe4/0x298 > [ 52.642599] c1 [] mutex_lock_nested+0x7c/0x3cc > [ 52.642604] c1 [] usb_gadget_unregister_driver+0x3c/0xc8 > [ 52.642608] c1 [] unregister_gadget_item+0x28/0x44 > [ 52.642613] c1 [] ffs_data_clear+0x138/0x140 > [ 52.642618] c1 [] ffs_data_reset+0x20/0x6c > [ 52.642621] c1 [] ffs_data_closed+0xac/0x12c > [ 52.642625] c1 [] ffs_ep0_release+0x20/0x2c > [ 52.642629] c1 [] __fput+0xb0/0x1f4 > [ 52.642633] c1 [] ____fput+0x20/0x2c > [ 52.642636] c1 [] task_work_run+0xb4/0xe8 > [ 52.642640] c1 [] do_exit+0x360/0xb9c > [ 52.642644] c1 [] do_group_exit+0x4c/0xb0 > [ 52.642647] c1 [] get_signal+0x380/0x89c > [ 52.642651] c1 [] do_signal+0x154/0x518 > [ 52.642656] c1 [] do_notify_resume+0x70/0x78 > [ 52.642659] c1 [] work_pending+0x1c/0x20 > > Signed-off-by: Baolin Wang Acked-by: Michal Nazarewicz > --- > drivers/usb/gadget/function/f_fs.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > index 0780d83..93de3b9 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -3666,6 +3666,7 @@ static void ffs_closed(struct ffs_data *ffs) > { > struct ffs_dev *ffs_obj; > struct f_fs_opts *opts; > + struct config_item *ci; > > ENTER(); > ffs_dev_lock(); > @@ -3689,8 +3690,11 @@ static void ffs_closed(struct ffs_data *ffs) > || !atomic_read(&opts->func_inst.group.cg_item.ci_kref.refcount)) > goto done; > > - unregister_gadget_item(ffs_obj->opts-> > - func_inst.group.cg_item.ci_parent->ci_parent); > + ci = opts->func_inst.group.cg_item.ci_parent->ci_parent; > + ffs_dev_unlock(); > + > + unregister_gadget_item(ci); > + return; > done: > ffs_dev_unlock(); > } > -- > 1.7.9.5 > -- Best regards ミハウ “????????????????86” ナザレヴイツ «If at first you don’t succeed, give up skydiving»