* [PATCH] usb: gadget: f_fs: Remove WARN_ON in functionfs_bind [not found] <CGME20241219125248epcas5p3887188e4df29b7b580cce9cfe6fed79f@epcas5p3.samsung.com> @ 2024-12-19 12:52 ` Akash M 2024-12-24 6:42 ` Akash M/Akash M 0 siblings, 1 reply; 3+ messages in thread From: Akash M @ 2024-12-19 12:52 UTC (permalink / raw) To: gregkh, paul, Chris.Wulff, tudor.ambarus, m.grzeschik, viro, quic_jjohnson, linux-usb, linux-kernel Cc: jh0801.jung, dh10.jung, naushad, akash.m5, rc93.raju, taehyun.cho, hongpooh.kim, eomji.oh, shijie.cai, alim.akhtar, selvarasu.g, stable This commit addresses an issue related to below kernel panic where panic_on_warn is enabled. It is caused by the unnecessary use of WARN_ON in functionsfs_bind, which easily leads to the following scenarios. 1.adb_write in adbd 2. UDC write via configfs ================= ===================== ->usb_ffs_open_thread() ->UDC write ->open_functionfs() ->configfs_write_iter() ->adb_open() ->gadget_dev_desc_UDC_store() ->adb_write() ->usb_gadget_register_driver_owner ->driver_register() ->StartMonitor() ->bus_add_driver() ->adb_read() ->gadget_bind_driver() <times-out without BIND event> ->configfs_composite_bind() ->usb_add_function() ->open_functionfs() ->ffs_func_bind() ->adb_open() ->functionfs_bind() <ffs->state !=FFS_ACTIVE> The adb_open, adb_read, and adb_write operations are invoked from the daemon, but trying to bind the function is a process that is invoked by UDC write through configfs, which opens up the possibility of a race condition between the two paths. In this race scenario, the kernel panic occurs due to the WARN_ON from functionfs_bind when panic_on_warn is enabled. This commit fixes the kernel panic by removing the unnecessary WARN_ON. Kernel panic - not syncing: kernel: panic_on_warn set ... [ 14.542395] Call trace: [ 14.542464] ffs_func_bind+0x1c8/0x14a8 [ 14.542468] usb_add_function+0xcc/0x1f0 [ 14.542473] configfs_composite_bind+0x468/0x588 [ 14.542478] gadget_bind_driver+0x108/0x27c [ 14.542483] really_probe+0x190/0x374 [ 14.542488] __driver_probe_device+0xa0/0x12c [ 14.542492] driver_probe_device+0x3c/0x220 [ 14.542498] __driver_attach+0x11c/0x1fc [ 14.542502] bus_for_each_dev+0x104/0x160 [ 14.542506] driver_attach+0x24/0x34 [ 14.542510] bus_add_driver+0x154/0x270 [ 14.542514] driver_register+0x68/0x104 [ 14.542518] usb_gadget_register_driver_owner+0x48/0xf4 [ 14.542523] gadget_dev_desc_UDC_store+0xf8/0x144 [ 14.542526] configfs_write_iter+0xf0/0x138 Fixes: ddf8abd25994 ("USB: f_fs: the FunctionFS driver") Cc: stable@vger.kernel.org Signed-off-by: Akash M <akash.m5@samsung.com> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 2920f8000bbd..92c883440e02 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -2285,7 +2285,7 @@ static int functionfs_bind(struct ffs_data *ffs, struct usb_composite_dev *cdev) struct usb_gadget_strings **lang; int first_id; - if (WARN_ON(ffs->state != FFS_ACTIVE + if ((ffs->state != FFS_ACTIVE || test_and_set_bit(FFS_FL_BOUND, &ffs->flags))) return -EBADFD; -- 2.17.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: gadget: f_fs: Remove WARN_ON in functionfs_bind 2024-12-19 12:52 ` [PATCH] usb: gadget: f_fs: Remove WARN_ON in functionfs_bind Akash M @ 2024-12-24 6:42 ` Akash M/Akash M 2024-12-24 7:38 ` Greg KH 0 siblings, 1 reply; 3+ messages in thread From: Akash M/Akash M @ 2024-12-24 6:42 UTC (permalink / raw) To: gregkh, paul, Chris.Wulff, tudor.ambarus, m.grzeschik, viro, quic_jjohnson, linux-usb, linux-kernel Cc: jh0801.jung, dh10.jung, naushad, rc93.raju, taehyun.cho, hongpooh.kim, eomji.oh, shijie.cai, alim.akhtar, selvarasu.g, stable On 12/19/2024 6:22 PM, Akash M wrote: > This commit addresses an issue related to below kernel panic where > panic_on_warn is enabled. It is caused by the unnecessary use of WARN_ON > in functionsfs_bind, which easily leads to the following scenarios. > > 1.adb_write in adbd 2. UDC write via configfs > ================= ===================== > > ->usb_ffs_open_thread() ->UDC write > ->open_functionfs() ->configfs_write_iter() > ->adb_open() ->gadget_dev_desc_UDC_store() > ->adb_write() ->usb_gadget_register_driver_owner > ->driver_register() > ->StartMonitor() ->bus_add_driver() > ->adb_read() ->gadget_bind_driver() > <times-out without BIND event> ->configfs_composite_bind() > ->usb_add_function() > ->open_functionfs() ->ffs_func_bind() > ->adb_open() ->functionfs_bind() > <ffs->state !=FFS_ACTIVE> > > The adb_open, adb_read, and adb_write operations are invoked from the > daemon, but trying to bind the function is a process that is invoked by > UDC write through configfs, which opens up the possibility of a race > condition between the two paths. In this race scenario, the kernel panic > occurs due to the WARN_ON from functionfs_bind when panic_on_warn is > enabled. This commit fixes the kernel panic by removing the unnecessary > WARN_ON. > > Kernel panic - not syncing: kernel: panic_on_warn set ... > [ 14.542395] Call trace: > [ 14.542464] ffs_func_bind+0x1c8/0x14a8 > [ 14.542468] usb_add_function+0xcc/0x1f0 > [ 14.542473] configfs_composite_bind+0x468/0x588 > [ 14.542478] gadget_bind_driver+0x108/0x27c > [ 14.542483] really_probe+0x190/0x374 > [ 14.542488] __driver_probe_device+0xa0/0x12c > [ 14.542492] driver_probe_device+0x3c/0x220 > [ 14.542498] __driver_attach+0x11c/0x1fc > [ 14.542502] bus_for_each_dev+0x104/0x160 > [ 14.542506] driver_attach+0x24/0x34 > [ 14.542510] bus_add_driver+0x154/0x270 > [ 14.542514] driver_register+0x68/0x104 > [ 14.542518] usb_gadget_register_driver_owner+0x48/0xf4 > [ 14.542523] gadget_dev_desc_UDC_store+0xf8/0x144 > [ 14.542526] configfs_write_iter+0xf0/0x138 > > Fixes: ddf8abd25994 ("USB: f_fs: the FunctionFS driver") > Cc: stable@vger.kernel.org > Signed-off-by: Akash M <akash.m5@samsung.com> > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > index 2920f8000bbd..92c883440e02 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -2285,7 +2285,7 @@ static int functionfs_bind(struct ffs_data *ffs, struct usb_composite_dev *cdev) > struct usb_gadget_strings **lang; > int first_id; > > - if (WARN_ON(ffs->state != FFS_ACTIVE > + if ((ffs->state != FFS_ACTIVE > || test_and_set_bit(FFS_FL_BOUND, &ffs->flags))) > return -EBADFD; > Hi Greg, I realized there's a minor nitpick with the patch I submitted - specifically a pair of extra brackets not removed. Do you want me to proceed with sending a v2 to address this, or is this something you can take care while applying this patch? Looking forward to your advice. Thanks, Akash ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: gadget: f_fs: Remove WARN_ON in functionfs_bind 2024-12-24 6:42 ` Akash M/Akash M @ 2024-12-24 7:38 ` Greg KH 0 siblings, 0 replies; 3+ messages in thread From: Greg KH @ 2024-12-24 7:38 UTC (permalink / raw) To: Akash M/Akash M Cc: paul, Chris.Wulff, tudor.ambarus, m.grzeschik, viro, quic_jjohnson, linux-usb, linux-kernel, jh0801.jung, dh10.jung, naushad, rc93.raju, taehyun.cho, hongpooh.kim, eomji.oh, shijie.cai, alim.akhtar, selvarasu.g, stable On Tue, Dec 24, 2024 at 12:12:49PM +0530, Akash M/Akash M wrote: > > On 12/19/2024 6:22 PM, Akash M wrote: > > This commit addresses an issue related to below kernel panic where > > panic_on_warn is enabled. It is caused by the unnecessary use of WARN_ON > > in functionsfs_bind, which easily leads to the following scenarios. > > > > 1.adb_write in adbd 2. UDC write via configfs > > ================= ===================== > > > > ->usb_ffs_open_thread() ->UDC write > > ->open_functionfs() ->configfs_write_iter() > > ->adb_open() ->gadget_dev_desc_UDC_store() > > ->adb_write() ->usb_gadget_register_driver_owner > > ->driver_register() > > ->StartMonitor() ->bus_add_driver() > > ->adb_read() ->gadget_bind_driver() > > <times-out without BIND event> ->configfs_composite_bind() > > ->usb_add_function() > > ->open_functionfs() ->ffs_func_bind() > > ->adb_open() ->functionfs_bind() > > <ffs->state !=FFS_ACTIVE> > > > > The adb_open, adb_read, and adb_write operations are invoked from the > > daemon, but trying to bind the function is a process that is invoked by > > UDC write through configfs, which opens up the possibility of a race > > condition between the two paths. In this race scenario, the kernel panic > > occurs due to the WARN_ON from functionfs_bind when panic_on_warn is > > enabled. This commit fixes the kernel panic by removing the unnecessary > > WARN_ON. > > > > Kernel panic - not syncing: kernel: panic_on_warn set ... > > [ 14.542395] Call trace: > > [ 14.542464] ffs_func_bind+0x1c8/0x14a8 > > [ 14.542468] usb_add_function+0xcc/0x1f0 > > [ 14.542473] configfs_composite_bind+0x468/0x588 > > [ 14.542478] gadget_bind_driver+0x108/0x27c > > [ 14.542483] really_probe+0x190/0x374 > > [ 14.542488] __driver_probe_device+0xa0/0x12c > > [ 14.542492] driver_probe_device+0x3c/0x220 > > [ 14.542498] __driver_attach+0x11c/0x1fc > > [ 14.542502] bus_for_each_dev+0x104/0x160 > > [ 14.542506] driver_attach+0x24/0x34 > > [ 14.542510] bus_add_driver+0x154/0x270 > > [ 14.542514] driver_register+0x68/0x104 > > [ 14.542518] usb_gadget_register_driver_owner+0x48/0xf4 > > [ 14.542523] gadget_dev_desc_UDC_store+0xf8/0x144 > > [ 14.542526] configfs_write_iter+0xf0/0x138 > > > > Fixes: ddf8abd25994 ("USB: f_fs: the FunctionFS driver") > > Cc: stable@vger.kernel.org > > Signed-off-by: Akash M <akash.m5@samsung.com> > > > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > > index 2920f8000bbd..92c883440e02 100644 > > --- a/drivers/usb/gadget/function/f_fs.c > > +++ b/drivers/usb/gadget/function/f_fs.c > > @@ -2285,7 +2285,7 @@ static int functionfs_bind(struct ffs_data *ffs, struct usb_composite_dev *cdev) > > struct usb_gadget_strings **lang; > > int first_id; > > > > - if (WARN_ON(ffs->state != FFS_ACTIVE > > + if ((ffs->state != FFS_ACTIVE > > || test_and_set_bit(FFS_FL_BOUND, &ffs->flags))) > > return -EBADFD; > > > Hi Greg, > > I realized there's a minor nitpick with the patch I submitted - > specifically a pair of extra brackets not removed. > > Do you want me to proceed with sending a v2 to address this, or is this > something you can take care while applying this patch? It's already in my tree, as you should have gotten an email about that. Just send a cleanup patch for later, it's not a big deal. thanks, greg k-h ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-12-24 7:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20241219125248epcas5p3887188e4df29b7b580cce9cfe6fed79f@epcas5p3.samsung.com>
2024-12-19 12:52 ` [PATCH] usb: gadget: f_fs: Remove WARN_ON in functionfs_bind Akash M
2024-12-24 6:42 ` Akash M/Akash M
2024-12-24 7:38 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox