Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 23 May 2022 16:03:32 +0800 (GMT+08:00) | From | duoming@zju ... | Subject | Re: [PATCH v3] mwifiex: fix sleep in atomic context bugs caused by dev_coredumpv |
| |
Hello,
On Mon, 23 May 2022 08:31:46 +0200 Greg KH wrote:
> > There are sleep in atomic context bugs when uploading device dump > > data in mwifiex. The root cause is that dev_coredumpv could not > > be used in atomic contexts, because it calls dev_set_name which > > include operations that may sleep. The call tree shows execution > > paths that could lead to bugs: > > > > (Interrupt context) > > fw_dump_timer_fn > > mwifiex_upload_device_dump > > dev_coredumpv(..., GFP_KERNEL) > > dev_coredumpm() > > kzalloc(sizeof(*devcd), gfp); //may sleep > > dev_set_name > > kobject_set_name_vargs > > kvasprintf_const(GFP_KERNEL, ...); //may sleep > > kstrdup(s, GFP_KERNEL); //may sleep > > > > In order to let dev_coredumpv support atomic contexts, this patch > > changes the gfp_t parameter of kvasprintf_const and kstrdup in > > kobject_set_name_vargs from GFP_KERNEL to GFP_ATOMIC. What's more, > > In order to mitigate bug, this patch changes the gfp_t parameter > > of dev_coredumpv from GFP_KERNEL to GFP_ATOMIC. > > > > Fixes: 57670ee882d4 ("mwifiex: device dump support via devcoredump framework") > > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn> > > --- > > Changes in v3: > > - Let dev_coredumpv support atomic contexts. > > > > drivers/net/wireless/marvell/mwifiex/main.c | 2 +- > > lib/kobject.c | 4 ++-- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c > > index ace7371c477..258906920a2 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/main.c > > +++ b/drivers/net/wireless/marvell/mwifiex/main.c > > @@ -1116,7 +1116,7 @@ void mwifiex_upload_device_dump(struct mwifiex_adapter *adapter) > > mwifiex_dbg(adapter, MSG, > > "== mwifiex dump information to /sys/class/devcoredump start\n"); > > dev_coredumpv(adapter->dev, adapter->devdump_data, adapter->devdump_len, > > - GFP_KERNEL); > > + GFP_ATOMIC); > > mwifiex_dbg(adapter, MSG, > > "== mwifiex dump information to /sys/class/devcoredump end\n"); > > > > diff --git a/lib/kobject.c b/lib/kobject.c > > index 5f0e71ab292..7672c54944c 100644 > > --- a/lib/kobject.c > > +++ b/lib/kobject.c > > @@ -254,7 +254,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt, > > if (kobj->name && !fmt) > > return 0; > > > > - s = kvasprintf_const(GFP_KERNEL, fmt, vargs); > > + s = kvasprintf_const(GFP_ATOMIC, fmt, vargs); > > if (!s) > > return -ENOMEM; > > > > @@ -267,7 +267,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt, > > if (strchr(s, '/')) { > > char *t; > > > > - t = kstrdup(s, GFP_KERNEL); > > + t = kstrdup(s, GFP_ATOMIC); > > kfree_const(s); > > if (!t) > > return -ENOMEM; > > Please no, you are hurting the whole kernel because of one odd user. > Please do not make these calls under atomic context.
Thanks for your time and suggestions. I will remove the gfp_t parameter of dev_coredumpv in order to show it could not be used in atomic context.
Best Regards, Duoming Zhou | ![\](/images/icornerr.gif) |