2022-09-26 08:03:42

by Duoming Zhou

[permalink] [raw]
Subject: [PATCH V10] devcoredump: add context check in dev_coredumpm

The dev_coredumpm(), dev_coredumpv() and dev_coredumpsg() could not
be used in atomic context, because they call kvasprintf_const() and
kstrdup() with GFP_KERNEL parameter. The process is shown below:

dev_coredumpv(.., gfp_t gfp)
dev_coredumpm(.., gfp_t gfp)
dev_set_name
kobject_set_name_vargs
kvasprintf_const(GFP_KERNEL, ...); //may sleep
kstrdup(s, GFP_KERNEL); //may sleep

This patch adds context check in dev_coredumpm() in order to show
dev_coredumpm() and its callers could not be used in atomic context.

What's more, this change can allow the api to evolve and will not
influence the users that call this api.

Signed-off-by: Duoming Zhou <[email protected]>
---
Changes since v9:
- v9: Add context check in dev_coredumpm().
- v10: Remove the "Fixes" tag.

drivers/base/devcoredump.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index f4d794d6bb8..806ee872f5f 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -255,6 +255,9 @@ void dev_coredumpm(struct device *dev, struct module *owner,
struct devcd_entry *devcd;
struct device *existing;

+ if (!gfpflags_normal_context(gfp))
+ goto free;
+
if (devcd_disabled)
goto free;

--
2.17.1


2022-09-26 08:50:06

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH V10] devcoredump: add context check in dev_coredumpm

On Mon, Sep 26, 2022 at 02:51:26PM +0800, Duoming Zhou wrote:
> The dev_coredumpm(), dev_coredumpv() and dev_coredumpsg() could not
> be used in atomic context, because they call kvasprintf_const() and
> kstrdup() with GFP_KERNEL parameter. The process is shown below:
>
> dev_coredumpv(.., gfp_t gfp)
> dev_coredumpm(.., gfp_t gfp)
> dev_set_name
> kobject_set_name_vargs
> kvasprintf_const(GFP_KERNEL, ...); //may sleep
> kstrdup(s, GFP_KERNEL); //may sleep
>
> This patch adds context check in dev_coredumpm() in order to show
> dev_coredumpm() and its callers could not be used in atomic context.
>
> What's more, this change can allow the api to evolve and will not
> influence the users that call this api.
>
> Signed-off-by: Duoming Zhou <[email protected]>
> ---
> Changes since v9:
> - v9: Add context check in dev_coredumpm().
> - v10: Remove the "Fixes" tag.
>
> drivers/base/devcoredump.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
> index f4d794d6bb8..806ee872f5f 100644
> --- a/drivers/base/devcoredump.c
> +++ b/drivers/base/devcoredump.c
> @@ -255,6 +255,9 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> struct devcd_entry *devcd;
> struct device *existing;
>
> + if (!gfpflags_normal_context(gfp))
> + goto free;

I still do not know why this is needed at all as there are no in-kernel
abusers of this api.

We do not add code to the kernel for out-of-tree users, or "future
users" at all, otherwise it would be unmaintable.

thanks,

greg k-h