LinuxPPC-Dev Archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] soc: fsl: guts: Revert commit 3c0d64e867ed
@ 2021-11-03 20:00 Christophe JAILLET
  2021-11-03 20:00 ` [PATCH 2/2] soc: fsl: guts: Add a missing memory allocation failure check Christophe JAILLET
  2022-01-07  2:36 ` [PATCH 1/2] soc: fsl: guts: Revert commit 3c0d64e867ed Li Yang
  0 siblings, 2 replies; 5+ messages in thread
From: Christophe JAILLET @ 2021-11-03 20:00 UTC (permalink / raw
  To: leoyang.li, tyreld
  Cc: kernel-janitors, Christophe JAILLET, linuxppc-dev, linux-kernel,
	linux-arm-kernel

This reverts commit 3c0d64e867ed
("soc: fsl: guts: reuse machine name from device tree").

A following patch will fix the missing memory allocation failure check
instead.

Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This is a follow-up of discussion in:
https://lore.kernel.org/kernel-janitors/b12e8c5c5d6ab3061d9504de8fbaefcad6bbc385.1629321668.git.christophe.jaillet@wanadoo.fr/
---
 drivers/soc/fsl/guts.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 072473a16f4d..af7741eafc57 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -28,7 +28,6 @@ struct fsl_soc_die_attr {
 static struct guts *guts;
 static struct soc_device_attribute soc_dev_attr;
 static struct soc_device *soc_dev;
-static struct device_node *root;
 
 
 /* SoC die attribute definition for QorIQ platform */
@@ -138,7 +137,7 @@ static u32 fsl_guts_get_svr(void)
 
 static int fsl_guts_probe(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *root, *np = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
 	const struct fsl_soc_die_attr *soc_die;
 	const char *machine;
@@ -159,8 +158,9 @@ static int fsl_guts_probe(struct platform_device *pdev)
 	root = of_find_node_by_path("/");
 	if (of_property_read_string(root, "model", &machine))
 		of_property_read_string_index(root, "compatible", 0, &machine);
+	of_node_put(root);
 	if (machine)
-		soc_dev_attr.machine = machine;
+		soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);
 
 	svr = fsl_guts_get_svr();
 	soc_die = fsl_soc_die_match(svr, fsl_soc_die);
@@ -195,7 +195,6 @@ static int fsl_guts_probe(struct platform_device *pdev)
 static int fsl_guts_remove(struct platform_device *dev)
 {
 	soc_device_unregister(soc_dev);
-	of_node_put(root);
 	return 0;
 }
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] soc: fsl: guts: Add a missing memory allocation failure check
  2021-11-03 20:00 [PATCH 1/2] soc: fsl: guts: Revert commit 3c0d64e867ed Christophe JAILLET
@ 2021-11-03 20:00 ` Christophe JAILLET
  2021-11-04  6:03   ` Michael Ellerman
  2022-01-07  2:40   ` Li Yang
  2022-01-07  2:36 ` [PATCH 1/2] soc: fsl: guts: Revert commit 3c0d64e867ed Li Yang
  1 sibling, 2 replies; 5+ messages in thread
From: Christophe JAILLET @ 2021-11-03 20:00 UTC (permalink / raw
  To: leoyang.li, tyreld
  Cc: kernel-janitors, Christophe JAILLET, linuxppc-dev, linux-kernel,
	linux-arm-kernel

If 'devm_kstrdup()' fails, we should return -ENOMEM.

While at it, move the 'of_node_put()' call in the error handling path and
after the 'machine' has been copied.
Better safe than sorry.

Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Not sure of which Fixes tag to add. Should be a6fc3b698130, but since
another commit needs to be reverted for this patch to make sense, I'm
unsure of what to do. :(
So, none is given.
---
 drivers/soc/fsl/guts.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index af7741eafc57..5ed2fc1c53a0 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -158,9 +158,14 @@ static int fsl_guts_probe(struct platform_device *pdev)
 	root = of_find_node_by_path("/");
 	if (of_property_read_string(root, "model", &machine))
 		of_property_read_string_index(root, "compatible", 0, &machine);
-	of_node_put(root);
-	if (machine)
+	if (machine) {
 		soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);
+		if (!soc_dev_attr.machine) {
+			of_node_put(root);
+			return -ENOMEM;
+		}
+	}
+	of_node_put(root);
 
 	svr = fsl_guts_get_svr();
 	soc_die = fsl_soc_die_match(svr, fsl_soc_die);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] soc: fsl: guts: Add a missing memory allocation failure check
  2021-11-03 20:00 ` [PATCH 2/2] soc: fsl: guts: Add a missing memory allocation failure check Christophe JAILLET
@ 2021-11-04  6:03   ` Michael Ellerman
  2022-01-07  2:40   ` Li Yang
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2021-11-04  6:03 UTC (permalink / raw
  To: Christophe JAILLET, leoyang.li, tyreld
  Cc: kernel-janitors, Christophe JAILLET, linuxppc-dev, linux-kernel,
	linux-arm-kernel

Christophe JAILLET <christophe.jaillet@wanadoo.fr> writes:
> If 'devm_kstrdup()' fails, we should return -ENOMEM.
>
> While at it, move the 'of_node_put()' call in the error handling path and
> after the 'machine' has been copied.
> Better safe than sorry.
>
> Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Not sure of which Fixes tag to add. Should be a6fc3b698130, but since
> another commit needs to be reverted for this patch to make sense, I'm
> unsure of what to do. :(
> So, none is given.

I think it's still correct to add:

  Fixes: a6fc3b698130 ("soc: fsl: add GUTS driver for QorIQ platforms")

That is where the bug was introduced, and adding the tag creates a link
between the fix and the bug, which is what we want.

The fact that it also requires the revert in order to apply is kind of
orthogonal, it means an automated backport of this commit will probably
fail, but that's OK it just means someone might have to do it manually.

There is some use of "Depends-on:" to flag a commit that is depended on,
but you can't use that in a patch submission because you don't know the
SHA of the parent commit.

Possibly whoever applies this can add a Depends-on: pointing to patch 1.

cheers

> ---
>  drivers/soc/fsl/guts.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index af7741eafc57..5ed2fc1c53a0 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -158,9 +158,14 @@ static int fsl_guts_probe(struct platform_device *pdev)
>  	root = of_find_node_by_path("/");
>  	if (of_property_read_string(root, "model", &machine))
>  		of_property_read_string_index(root, "compatible", 0, &machine);
> -	of_node_put(root);
> -	if (machine)
> +	if (machine) {
>  		soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);
> +		if (!soc_dev_attr.machine) {
> +			of_node_put(root);
> +			return -ENOMEM;
> +		}
> +	}
> +	of_node_put(root);
>  
>  	svr = fsl_guts_get_svr();
>  	soc_die = fsl_soc_die_match(svr, fsl_soc_die);
> -- 
> 2.30.2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] soc: fsl: guts: Revert commit 3c0d64e867ed
  2021-11-03 20:00 [PATCH 1/2] soc: fsl: guts: Revert commit 3c0d64e867ed Christophe JAILLET
  2021-11-03 20:00 ` [PATCH 2/2] soc: fsl: guts: Add a missing memory allocation failure check Christophe JAILLET
@ 2022-01-07  2:36 ` Li Yang
  1 sibling, 0 replies; 5+ messages in thread
From: Li Yang @ 2022-01-07  2:36 UTC (permalink / raw
  To: Christophe JAILLET
  Cc: tyreld, kernel-janitors, linuxppc-dev,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, lkml

On Thu, Nov 4, 2021 at 4:09 AM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> This reverts commit 3c0d64e867ed
> ("soc: fsl: guts: reuse machine name from device tree").
>
> A following patch will fix the missing memory allocation failure check
> instead.
>
> Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Applied for next.  Thanks.

> ---
> This is a follow-up of discussion in:
> https://lore.kernel.org/kernel-janitors/b12e8c5c5d6ab3061d9504de8fbaefcad6bbc385.1629321668.git.christophe.jaillet@wanadoo.fr/
> ---
>  drivers/soc/fsl/guts.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index 072473a16f4d..af7741eafc57 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -28,7 +28,6 @@ struct fsl_soc_die_attr {
>  static struct guts *guts;
>  static struct soc_device_attribute soc_dev_attr;
>  static struct soc_device *soc_dev;
> -static struct device_node *root;
>
>
>  /* SoC die attribute definition for QorIQ platform */
> @@ -138,7 +137,7 @@ static u32 fsl_guts_get_svr(void)
>
>  static int fsl_guts_probe(struct platform_device *pdev)
>  {
> -       struct device_node *np = pdev->dev.of_node;
> +       struct device_node *root, *np = pdev->dev.of_node;
>         struct device *dev = &pdev->dev;
>         const struct fsl_soc_die_attr *soc_die;
>         const char *machine;
> @@ -159,8 +158,9 @@ static int fsl_guts_probe(struct platform_device *pdev)
>         root = of_find_node_by_path("/");
>         if (of_property_read_string(root, "model", &machine))
>                 of_property_read_string_index(root, "compatible", 0, &machine);
> +       of_node_put(root);
>         if (machine)
> -               soc_dev_attr.machine = machine;
> +               soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);
>
>         svr = fsl_guts_get_svr();
>         soc_die = fsl_soc_die_match(svr, fsl_soc_die);
> @@ -195,7 +195,6 @@ static int fsl_guts_probe(struct platform_device *pdev)
>  static int fsl_guts_remove(struct platform_device *dev)
>  {
>         soc_device_unregister(soc_dev);
> -       of_node_put(root);
>         return 0;
>  }
>
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] soc: fsl: guts: Add a missing memory allocation failure check
  2021-11-03 20:00 ` [PATCH 2/2] soc: fsl: guts: Add a missing memory allocation failure check Christophe JAILLET
  2021-11-04  6:03   ` Michael Ellerman
@ 2022-01-07  2:40   ` Li Yang
  1 sibling, 0 replies; 5+ messages in thread
From: Li Yang @ 2022-01-07  2:40 UTC (permalink / raw
  To: Christophe JAILLET
  Cc: tyreld, kernel-janitors, linuxppc-dev,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, lkml

On Thu, Nov 4, 2021 at 4:10 AM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> If 'devm_kstrdup()' fails, we should return -ENOMEM.
>
> While at it, move the 'of_node_put()' call in the error handling path and
> after the 'machine' has been copied.
> Better safe than sorry.
>
> Suggested-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Applied with Fixes tag and Depends-on tag added.  Thanks.

> ---
> Not sure of which Fixes tag to add. Should be a6fc3b698130, but since
> another commit needs to be reverted for this patch to make sense, I'm
> unsure of what to do. :(
> So, none is given.
> ---
>  drivers/soc/fsl/guts.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index af7741eafc57..5ed2fc1c53a0 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -158,9 +158,14 @@ static int fsl_guts_probe(struct platform_device *pdev)
>         root = of_find_node_by_path("/");
>         if (of_property_read_string(root, "model", &machine))
>                 of_property_read_string_index(root, "compatible", 0, &machine);
> -       of_node_put(root);
> -       if (machine)
> +       if (machine) {
>                 soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);
> +               if (!soc_dev_attr.machine) {
> +                       of_node_put(root);
> +                       return -ENOMEM;
> +               }
> +       }
> +       of_node_put(root);
>
>         svr = fsl_guts_get_svr();
>         soc_die = fsl_soc_die_match(svr, fsl_soc_die);
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-01-07  2:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-03 20:00 [PATCH 1/2] soc: fsl: guts: Revert commit 3c0d64e867ed Christophe JAILLET
2021-11-03 20:00 ` [PATCH 2/2] soc: fsl: guts: Add a missing memory allocation failure check Christophe JAILLET
2021-11-04  6:03   ` Michael Ellerman
2022-01-07  2:40   ` Li Yang
2022-01-07  2:36 ` [PATCH 1/2] soc: fsl: guts: Revert commit 3c0d64e867ed Li Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).