1. Patchew
  2. linux
  3. View series

[PATCH V9] devcoredump: add context check in dev_coredumpm

Duoming Zhou posted 1 patch 3 years, 6 months ago
There is a newer version of this series
drivers/base/devcoredump.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH V9] devcoredump: add context check in dev_coredumpm
Posted by Duoming Zhou 3 years, 6 months ago 
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.

Fixes: 833c95456a70 ("device coredump: add new device coredump class")
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
Changes in v9:
  - Add context check in dev_coredumpm().

 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
Re: [PATCH V9] devcoredump: add context check in dev_coredumpm
Posted by Greg KH 3 years, 6 months ago 
On Mon, Sep 26, 2022 at 02:16:09PM +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.
> 
> Fixes: 833c95456a70 ("device coredump: add new device coredump class")
> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> ---
> Changes in v9:
>   - Add context check in dev_coredumpm().
> 
>  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;

Wait, this just broke things for no good reason if a caller happened to
have set a GFP flag that we do not like at the moment.

What code now does not work that previously did work with this change?

thanks,

greg k-h
Re: [PATCH V9] devcoredump: add context check in dev_coredumpm
Posted by duoming@zju.edu.cn 3 years, 6 months ago 
Hello,

On Mon, 26 Sep 2022 08:27:58 +0200 Greg KH wrote:

> On Mon, Sep 26, 2022 at 02:16:09PM +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.
> > 
> > Fixes: 833c95456a70 ("device coredump: add new device coredump class")
> > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> > ---
> > Changes in v9:
> >   - Add context check in dev_coredumpm().
> > 
> >  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;
> 
> Wait, this just broke things for no good reason if a caller happened to
> have set a GFP flag that we do not like at the moment.
> 
> What code now does not work that previously did work with this change?

I found that all users in the kernel call the dev_coredumpv(), dev_coredumpm() and
dev_coredumpsg() with "GFP_KERNEL". So this change will not influence the existing users.

Best regards,
Duoming Zhou
Re: [PATCH V9] devcoredump: add context check in dev_coredumpm
Posted by Greg KH 3 years, 6 months ago 
On Mon, Sep 26, 2022 at 02:38:28PM +0800, duoming@zju.edu.cn wrote:
> Hello,
> 
> On Mon, 26 Sep 2022 08:27:58 +0200 Greg KH wrote:
> 
> > On Mon, Sep 26, 2022 at 02:16:09PM +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.
> > > 
> > > Fixes: 833c95456a70 ("device coredump: add new device coredump class")
> > > Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> > > ---
> > > Changes in v9:
> > >   - Add context check in dev_coredumpm().
> > > 
> > >  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;
> > 
> > Wait, this just broke things for no good reason if a caller happened to
> > have set a GFP flag that we do not like at the moment.
> > 
> > What code now does not work that previously did work with this change?
> 
> I found that all users in the kernel call the dev_coredumpv(), dev_coredumpm() and
> dev_coredumpsg() with "GFP_KERNEL". So this change will not influence the existing users.

Great, so there is no need for this, and it does not "fix" any commit.

confused,

greg k-h

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.