[PATCH resent] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext

Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH resent] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup()
       [not found] ` <3675f707-bff0-3caf-29a2-b99e5b9c6554@web.de>
@ 2023-03-25  9:43   ` Markus Elfring
  2025-03-04 19:21   ` [PATCH RESEND] " Markus Elfring
  1 sibling, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2023-03-25  9:43 UTC (permalink / raw)
  To: kernel-janitors, linux-mtd, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 20:13:51 +0100

The label “setup_err” was used to jump to another pointer check despite of
the detail in the implementation of the function “cfi_intelext_setup”
that it was determined already that a corresponding variable contained
a null pointer because of a failed memory allocation.

* Thus use more appropriate labels instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 54f92d09d9cf..a06318cd5ea4 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -614,7 +614,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 				    sizeof(struct mtd_erase_region_info),
 				    GFP_KERNEL);
 	if (!mtd->eraseregions)
-		goto setup_err;
+		goto free_mtd;

 	for (i=0; i<cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
@@ -630,7 +630,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].numblocks = ernum;
 			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap = kmalloc(ernum / 8 + 1, GFP_KERNEL);
 			if (!mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap)
-				goto setup_err;
+				goto release_loop;
 		}
 		offset += (ersize * ernum);
 	}
@@ -638,7 +638,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	if (offset != devsize) {
 		/* Argh */
 		printk(KERN_WARNING "Sum of regions (%lx) != total size of set of interleaved chips (%lx)\n", offset, devsize);
-		goto setup_err;
+		goto release_loop;
 	}

 	for (i=0; i<mtd->numeraseregions;i++){
@@ -660,18 +660,18 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	/* This function has the potential to distort the reality
 	   a bit and therefore should be called last. */
 	if (cfi_intelext_partition_fixup(mtd, &cfi) != 0)
-		goto setup_err;
+		goto release_loop;

 	__module_get(THIS_MODULE);
 	register_reboot_notifier(&mtd->reboot_notifier);
 	return mtd;

- setup_err:
-	if (mtd->eraseregions)
-		for (i=0; i<cfi->cfiq->NumEraseRegions; i++)
-			for (j=0; j<cfi->numchips; j++)
-				kfree(mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap);
+release_loop:
+	for (i=0; i<cfi->cfiq->NumEraseRegions; i++)
+		for (j=0; j<cfi->numchips; j++)
+			kfree(mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap);
 	kfree(mtd->eraseregions);
+free_mtd:
 	kfree(mtd);
 	kfree(cfi->cmdset_priv);
 	return NULL;
--
2.40.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH RESEND] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup()
       [not found] ` <3675f707-bff0-3caf-29a2-b99e5b9c6554@web.de>
  2023-03-25  9:43   ` [PATCH resent] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup() Markus Elfring
@ 2025-03-04 19:21   ` Markus Elfring
  2025-03-18 10:26     ` Miquel Raynal
  2025-03-18 12:03     ` [PATCH RESEND] " Dan Carpenter
  1 sibling, 2 replies; 5+ messages in thread
From: Markus Elfring @ 2025-03-04 19:21 UTC (permalink / raw)
  To: kernel-janitors, linux-mtd, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: cocci, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 21 Mar 2023 20:13:51 +0100

The label “setup_err” was used to jump to another pointer check despite of
the detail in the implementation of the function “cfi_intelext_setup”
that it was determined already that a corresponding variable contained
a null pointer because of a failed memory allocation.

* Thus use more appropriate labels instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 54f92d09d9cf..a06318cd5ea4 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -614,7 +614,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 				    sizeof(struct mtd_erase_region_info),
 				    GFP_KERNEL);
 	if (!mtd->eraseregions)
-		goto setup_err;
+		goto free_mtd;

 	for (i=0; i<cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
@@ -630,7 +630,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].numblocks = ernum;
 			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap = kmalloc(ernum / 8 + 1, GFP_KERNEL);
 			if (!mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap)
-				goto setup_err;
+				goto release_loop;
 		}
 		offset += (ersize * ernum);
 	}
@@ -638,7 +638,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	if (offset != devsize) {
 		/* Argh */
 		printk(KERN_WARNING "Sum of regions (%lx) != total size of set of interleaved chips (%lx)\n", offset, devsize);
-		goto setup_err;
+		goto release_loop;
 	}

 	for (i=0; i<mtd->numeraseregions;i++){
@@ -660,18 +660,18 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	/* This function has the potential to distort the reality
 	   a bit and therefore should be called last. */
 	if (cfi_intelext_partition_fixup(mtd, &cfi) != 0)
-		goto setup_err;
+		goto release_loop;

 	__module_get(THIS_MODULE);
 	register_reboot_notifier(&mtd->reboot_notifier);
 	return mtd;

- setup_err:
-	if (mtd->eraseregions)
-		for (i=0; i<cfi->cfiq->NumEraseRegions; i++)
-			for (j=0; j<cfi->numchips; j++)
-				kfree(mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap);
+release_loop:
+	for (i=0; i<cfi->cfiq->NumEraseRegions; i++)
+		for (j=0; j<cfi->numchips; j++)
+			kfree(mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap);
 	kfree(mtd->eraseregions);
+free_mtd:
 	kfree(mtd);
 	kfree(cfi->cmdset_priv);
 	return NULL;
--
2.40.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH RESEND] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup()
  2025-03-04 19:21   ` [PATCH RESEND] " Markus Elfring
@ 2025-03-18 10:26     ` Miquel Raynal
  2025-03-18 12:03       ` Markus Elfring
  2025-03-18 12:03     ` [PATCH RESEND] " Dan Carpenter
  1 sibling, 1 reply; 5+ messages in thread
From: Miquel Raynal @ 2025-03-18 10:26 UTC (permalink / raw)
  To: Markus Elfring
  Cc: kernel-janitors, linux-mtd, Richard Weinberger,
	Vignesh Raghavendra, cocci, LKML

Hi,

On 04/03/2025 at 20:21:53 +01, Markus Elfring <Markus.Elfring@web.de> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 21 Mar 2023 20:13:51 +0100
>
> The label “setup_err” was used to jump to another pointer check despite of
> the detail in the implementation of the function “cfi_intelext_setup”
> that it was determined already that a corresponding variable contained
> a null pointer because of a failed memory allocation.

Can you please rephrase the commit log? It is super hard to understand.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup()
  2025-03-18 10:26     ` Miquel Raynal
@ 2025-03-18 12:03       ` Markus Elfring
  0 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2025-03-18 12:03 UTC (permalink / raw)
  To: Miquel Raynal, linux-mtd
  Cc: kernel-janitors, LKML, cocci, Richard Weinberger,
	Vignesh Raghavendra

>> The label “setup_err” was used to jump to another pointer check despite of
>> the detail in the implementation of the function “cfi_intelext_setup”
>> that it was determined already that a corresponding variable contained
>> a null pointer because of a failed memory allocation.
>
> Can you please rephrase the commit log? It is super hard to understand.
Which wording variant would you find nicer for provided information?

* used jump targets

* null pointer assignments


Regards,
Markus

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH RESEND] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup()
  2025-03-04 19:21   ` [PATCH RESEND] " Markus Elfring
  2025-03-18 10:26     ` Miquel Raynal
@ 2025-03-18 12:03     ` Dan Carpenter
  1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-03-18 12:03 UTC (permalink / raw)
  To: Markus Elfring
  Cc: kernel-janitors, linux-mtd, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, cocci, LKML

On Tue, Mar 04, 2025 at 08:21:53PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 21 Mar 2023 20:13:51 +0100
> 
> The label “setup_err” was used to jump to another pointer check despite of
> the detail in the implementation of the function “cfi_intelext_setup”
> that it was determined already that a corresponding variable contained
> a null pointer because of a failed memory allocation.
> 
> * Thus use more appropriate labels instead.
> 
> * Delete a redundant check.
> 
> 
> This issue was detected by using the Coccinelle software.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Don't use a Fixes tag for cleanups.

regards,
dan carpenter


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, other threads:[~2025-03-18 12:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de>
     [not found] ` <3675f707-bff0-3caf-29a2-b99e5b9c6554@web.de>
2023-03-25  9:43   ` [PATCH resent] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup() Markus Elfring
2025-03-04 19:21   ` [PATCH RESEND] " Markus Elfring
2025-03-18 10:26     ` Miquel Raynal
2025-03-18 12:03       ` Markus Elfring
2025-03-18 12:03     ` [PATCH RESEND] " Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox