Duoming Zhou <[email protected]> writes:
> 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.
vmalloc in atomic context?
Not only does dev_coredumpm set a device name dev_coredumpm creates an
entire device to hold the device dump.
My sense is that either dev_coredumpm needs to be rebuilt on a
completely different principle that does not need a device to hold the
coredump (so that it can be called from interrupt context) or that
dev_coredumpm should never be called in an context that can not sleep.
Looking at fw_dump_timer_fn the only purpose of the timer is to trigger
a device dump after a certain amount of time. So I suspect all that is
needed to fix this issue is to change the type of devdump_timer to
struct delayed_work and use scheduled_delayed_work instead of mod_timer.
Eric
p.s. I looked at this because there was coredump in the infrastructure
name, and I do some of the work to keep coredumps working. Device dump
seems like a much better term, and I wished the designer of the api had
used that instead.