diff mbox series

[2/2] soc: fsl: guts: Add a missing memory allocation failure check

Message ID 4890990418ecbcfb8921efe8adb2019a03e5a1c1.1635969326.git.christophe.jaillet@wanadoo.fr (mailing list archive)
State New, archived
Headers show
Series [1/2] soc: fsl: guts: Revert commit 3c0d64e867ed | expand

Commit Message

Christophe JAILLET Nov. 3, 2021, 8 p.m. UTC
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(-)

Comments

Michael Ellerman Nov. 4, 2021, 6:03 a.m. UTC | #1
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
Leo Li Jan. 7, 2022, 2:40 a.m. UTC | #2
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
>
diff mbox series

Patch

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);