diff mbox series

diskpart: fix crash in case if GPT

Message ID 20200907090833.17970-1-sbabic@denx.de
State Accepted
Headers show
Series diskpart: fix crash in case if GPT | expand

Commit Message

Stefano Babic Sept. 7, 2020, 9:08 a.m. UTC
Switch to the same behavior as with MBR and rewrite completely the
partition table instead of trying to adjust a single entry.
The most reliable way to set new partitions seems to be to drop the old
table and to write from scratch the new table, without trying to adjust
previous entries. This is the way done for MBR, and this patch aligns
GPT in the same way.

Signed-off-by: Stefano Babic <sbabic@denx.de>
Reported-by: Heiko Schocher <hs@denx.de>
---
 handlers/diskpart_handler.c | 53 +++++++++++--------------------------
 1 file changed, 16 insertions(+), 37 deletions(-)

Comments

Heiko Schocher Sept. 7, 2020, 10:29 a.m. UTC | #1
Hello Stefano,

Am 07.09.2020 um 11:08 schrieb Stefano Babic:
> Switch to the same behavior as with MBR and rewrite completely the
> partition table instead of trying to adjust a single entry.
> The most reliable way to set new partitions seems to be to drop the old
> table and to write from scratch the new table, without trying to adjust
> previous entries. This is the way done for MBR, and this patch aligns
> GPT in the same way.
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> Reported-by: Heiko Schocher <hs@denx.de>
> ---
>   handlers/diskpart_handler.c | 53 +++++++++++--------------------------
>   1 file changed, 16 insertions(+), 37 deletions(-)

Thanks for this fix, this solved my problem, so:

Tested-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
diff mbox series

Patch

diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
index be570ca..5f4d777 100644
--- a/handlers/diskpart_handler.c
+++ b/handlers/diskpart_handler.c
@@ -243,48 +243,24 @@  static int diskpart(struct img_type *img,
 
 	i = 0;
 
-	if (priv.labeltype == FDISK_DISKLABEL_DOS) {
-		fdisk_delete_all_partitions(cxt);
-	}
+	fdisk_delete_all_partitions(cxt);
 
 	LIST_FOREACH(part, &priv.listparts, next) {
 		struct fdisk_partition *pa = NULL;
 		size_t partno;
+		struct fdisk_partition *newpa;
 
-		/*
-		 * Allow to have not consecutives partitions
-		 */
-		if (part->partno > i) {
-			while (i < part->partno) {
-				TRACE("DELETE PARTITION %d", i);
-				fdisk_delete_partition(cxt, i);
-				i++;
-			}
+		newpa = fdisk_new_partition();
+		ret = diskpart_set_partition(cxt, newpa, part);
+		if (ret) {
+			WARN("I cannot set all partition's parameters");
 		}
-
-		if (fdisk_get_partition(cxt, part->partno, &pa)) {
-			struct fdisk_partition *newpa;
-			newpa = fdisk_new_partition();
-			ret = diskpart_set_partition(cxt, newpa, part);
-			if (ret) {
-				WARN("I cannot set all partition's parameters");
-			}
-			if ((ret = fdisk_add_partition(cxt, newpa, &partno)) < 0) {
-				ERROR("I cannot add partition %zu(%s): %d", part->partno, part->name, ret);
-			}
-			fdisk_unref_partition(newpa);
-			if (ret < 0)
-				goto handler_exit;
-		} else {
-			ret = diskpart_set_partition(cxt, pa, part);
-			if (ret) {
-				WARN("I cannot set all partition's parameters");
-			}
-			if (fdisk_set_partition(cxt, part->partno, pa) < 0) {
-				ERROR("I cannot modify partition %zu(%s)", part->partno, part->name);
-			}
-			fdisk_unref_partition(pa);
+		if ((ret = fdisk_add_partition(cxt, newpa, &partno)) < 0) {
+			ERROR("I cannot add partition %zu(%s): %d", part->partno, part->name, ret);
 		}
+		fdisk_unref_partition(newpa);
+		if (ret < 0)
+			goto handler_exit;
 		fdisk_reset_partition(pa);
 		i++;
 	}
@@ -292,8 +268,11 @@  static int diskpart(struct img_type *img,
 	/*
 	 * Everything done, write into disk
 	 */
-	ret = fdisk_write_disklabel(cxt) |
-		fdisk_reread_partition_table(cxt);
+	ret = fdisk_write_disklabel(cxt);
+	if (ret)
+		ERROR("Partition table cannot be written on disk");
+	if (fdisk_reread_partition_table(cxt))
+		WARN("Table cannot be reread from the disk, be careful !");
 
 handler_exit:
 	if (fdisk_deassign_device(cxt, 0))