mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/3] partitions: dos: fix memory leaks
@ 2024-11-25 15:27 Ahmad Fatoum
  2024-11-25 15:27 ` [PATCH 2/3] partitions: efi: remove guid device parameter on free Ahmad Fatoum
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2024-11-25 15:27 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

These leaks don't bother us normally, but when fuzzing they quickly add
up, so let's fix them.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/partitions/dos.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/common/partitions/dos.c b/common/partitions/dos.c
index a388a23eed0d..5a055efec7d1 100644
--- a/common/partitions/dos.c
+++ b/common/partitions/dos.c
@@ -20,9 +20,16 @@
 #include <linux/err.h>
 #include <partitions.h>
 
+struct disk_signature_priv {
+	uint32_t signature;
+	struct block_device *blk;
+	struct param_d *param;
+};
+
 struct dos_partition_desc {
 	struct partition_desc pd;
 	uint32_t signature;
+	struct disk_signature_priv disksig;
 };
 
 struct dos_partition {
@@ -76,11 +83,6 @@ static int write_mbr(struct block_device *blk, void *buf)
 	return block_flush(blk);
 }
 
-struct disk_signature_priv {
-	uint32_t signature;
-	struct block_device *blk;
-};
-
 static int dos_set_disk_signature(struct param_d *p, void *_priv)
 {
 	struct disk_signature_priv *priv = _priv;
@@ -273,7 +275,7 @@ static struct partition_desc *dos_partition(void *buf, struct block_device *blk)
 	if (extended_partition)
 		dos_extended_partition(blk, dpd, extended_partition, signature);
 
-	dsp = xzalloc(sizeof(*dsp));
+	dsp = &dpd->disksig;
 	dsp->blk = blk;
 
 	/*
@@ -286,7 +288,7 @@ static struct partition_desc *dos_partition(void *buf, struct block_device *blk)
 	 * signature and pp is a zero-filled hex representation of the 1-based
 	 * partition number.
 	 */
-	dev_add_param_uint32(blk->dev, "nt_signature",
+	dsp->param = dev_add_param_uint32(blk->dev, "nt_signature",
 			dos_set_disk_signature, dos_get_disk_signature,
 			&dsp->signature, "%08x", dsp);
 
@@ -295,6 +297,8 @@ static struct partition_desc *dos_partition(void *buf, struct block_device *blk)
 
 static void dos_partition_free(struct partition_desc *pd)
 {
+	struct dos_partition_desc *dpd
+		= container_of(pd, struct dos_partition_desc, pd);
 	struct partition *part, *tmp;
 
 	list_for_each_entry_safe(part, tmp, &pd->partitions, list) {
@@ -303,6 +307,8 @@ static void dos_partition_free(struct partition_desc *pd)
 		free(dpart);
 	}
 
+	dev_remove_param(dpd->disksig.param);
+
 	free(pd);
 }
 
@@ -414,12 +420,16 @@ static __maybe_unused int dos_partition_mkpart(struct partition_desc *pd,
 	dpart = xzalloc(sizeof(*dpart));
 	part = &dpart->part;
 
-	if (start_lba > UINT_MAX)
+	if (start_lba > UINT_MAX) {
+		free(dpart);
 		return -ENOSPC;
+	}
 	size = end_lba - start_lba + 1;
 
-	if (size > UINT_MAX)
+	if (size > UINT_MAX) {
+		free(dpart);
 		return -ENOSPC;
+	}
 
 	dpart->type = fs_type_to_type(fs_type);
 
-- 
2.39.5




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

* [PATCH 2/3] partitions: efi: remove guid device parameter on free
  2024-11-25 15:27 [PATCH 1/3] partitions: dos: fix memory leaks Ahmad Fatoum
@ 2024-11-25 15:27 ` Ahmad Fatoum
  2024-11-25 15:27 ` [PATCH 3/3] partitions: efi: remove unnecessary NULLing of local variable Ahmad Fatoum
  2024-11-28  8:59 ` [PATCH 1/3] partitions: dos: fix memory leaks Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2024-11-25 15:27 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The leak doesn't bother us normally, but when fuzzing it quickly adds
up, so let's fix it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/partitions/efi.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index d4a7ee5d4471..968b959397b5 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -26,6 +26,7 @@ struct efi_partition_desc {
 	struct partition_desc pd;
 	gpt_header *gpt;
 	gpt_entry *ptes;
+	struct param_d *param_guid;
 };
 
 struct efi_partition {
@@ -513,9 +514,6 @@ static struct partition_desc *efi_partition(void *buf, struct block_device *blk)
 	if (!find_valid_gpt(buf, blk, &gpt, &ptes) || !gpt || !ptes)
 		return NULL;
 
-	snprintf(blk->cdev.diskuuid, sizeof(blk->cdev.diskuuid), "%pUl", &gpt->disk_guid);
-	dev_add_param_string_fixed(blk->dev, "guid", blk->cdev.diskuuid);
-
 	blk->cdev.flags |= DEVFS_IS_GPT_PARTITIONED;
 
 	nb_part = le32_to_cpu(gpt->num_partition_entries);
@@ -532,6 +530,10 @@ static struct partition_desc *efi_partition(void *buf, struct block_device *blk)
 	epd->gpt = gpt;
 	epd->ptes = ptes;
 
+	snprintf(blk->cdev.diskuuid, sizeof(blk->cdev.diskuuid), "%pUl", &gpt->disk_guid);
+	epd->param_guid = dev_add_param_string_fixed(blk->dev,
+						     "guid", blk->cdev.diskuuid);
+
 	for (i = 0; i < nb_part; i++) {
 		if (!is_pte_valid(&ptes[i], last_lba(blk))) {
 			dev_dbg(blk->dev, "Invalid pte %d\n", i);
@@ -566,6 +568,7 @@ static void efi_partition_free(struct partition_desc *pd)
 		free(epart);
 	}
 
+	dev_remove_param(epd->param_guid);
 	free(epd->ptes);
 	free(epd->gpt);
 	free(epd);
-- 
2.39.5




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

* [PATCH 3/3] partitions: efi: remove unnecessary NULLing of local variable
  2024-11-25 15:27 [PATCH 1/3] partitions: dos: fix memory leaks Ahmad Fatoum
  2024-11-25 15:27 ` [PATCH 2/3] partitions: efi: remove guid device parameter on free Ahmad Fatoum
@ 2024-11-25 15:27 ` Ahmad Fatoum
  2024-11-28  8:59 ` [PATCH 1/3] partitions: dos: fix memory leaks Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2024-11-25 15:27 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The variables are local and go out of scope anyway, so no point in
NULLing just before scope ends.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/partitions/efi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index 968b959397b5..af7eacfe3b68 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -122,7 +122,6 @@ static gpt_entry *alloc_read_gpt_entries(struct block_device *blk,
 	ret = block_read(blk, pte, from, size);
 	if (ret) {
 		free(pte);
-		pte=NULL;
 		return NULL;
 	}
 	return pte;
@@ -157,7 +156,6 @@ static gpt_header *alloc_read_gpt_header(struct block_device *blk,
 	ret = block_read(blk, gpt, lba, 1);
 	if (ret) {
 		free(gpt);
-		gpt=NULL;
 		return NULL;
 	}
 
-- 
2.39.5




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

* Re: [PATCH 1/3] partitions: dos: fix memory leaks
  2024-11-25 15:27 [PATCH 1/3] partitions: dos: fix memory leaks Ahmad Fatoum
  2024-11-25 15:27 ` [PATCH 2/3] partitions: efi: remove guid device parameter on free Ahmad Fatoum
  2024-11-25 15:27 ` [PATCH 3/3] partitions: efi: remove unnecessary NULLing of local variable Ahmad Fatoum
@ 2024-11-28  8:59 ` Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2024-11-28  8:59 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Mon, 25 Nov 2024 16:27:54 +0100, Ahmad Fatoum wrote:
> These leaks don't bother us normally, but when fuzzing they quickly add
> up, so let's fix them.
> 
> 

Applied, thanks!

[1/3] partitions: dos: fix memory leaks
      https://git.pengutronix.de/cgit/barebox/commit/?id=19f4033db59e (link may not be stable)
[2/3] partitions: efi: remove guid device parameter on free
      https://git.pengutronix.de/cgit/barebox/commit/?id=834cddeb4e91 (link may not be stable)
[3/3] partitions: efi: remove unnecessary NULLing of local variable
      https://git.pengutronix.de/cgit/barebox/commit/?id=afff0f4c98a5 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




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

end of thread, other threads:[~2024-11-28  8:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-25 15:27 [PATCH 1/3] partitions: dos: fix memory leaks Ahmad Fatoum
2024-11-25 15:27 ` [PATCH 2/3] partitions: efi: remove guid device parameter on free Ahmad Fatoum
2024-11-25 15:27 ` [PATCH 3/3] partitions: efi: remove unnecessary NULLing of local variable Ahmad Fatoum
2024-11-28  8:59 ` [PATCH 1/3] partitions: dos: fix memory leaks Sascha Hauer

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