diff mbox series

[v5] diskpart_handler: support explicitly sized partitions

Message ID 20220318151325.3142600-1-bkylerussell@gmail.com
State Accepted
Headers show
Series [v5] diskpart_handler: support explicitly sized partitions | expand

Commit Message

Kyle Russell March 18, 2022, 3:13 p.m. UTC 
When passing in a partition size in units of sectors, fdisk expects you to
enable explicit sizing on the partition; otherwise, fdisk assumes you want
the partition aligned to the nearest grain (typically 1MB), which may
corrupt a partition table containing already explicitly sized partitions.

This matches the behavior of other libfdisk clients in util-linux (see
excerpt from libfdisk's parse_line_nameval() below):

libfdisk/src/script.c:
        if (pow)        /* specified as <num><suffix> */
                num /= dp->cxt->sector_size;
        else            /* specified as number of sectors */
                fdisk_partition_size_explicit(pa, 1);

To achieve this, we allow ustrtoull() to return a char* to the suffix
of the converted string, if one exists.  If strtoull() points to a different
suffix in the string than ustrtoull(), we assume ustrtoull() consumed a
user provided size-type delimiter.  If no size-type delimiter match is found,
explicit sizing is not enabled and fdisk aligns the partition to the nearest
grain.

Signed-off-by: Kyle Russell <bkylerussell@gmail.com>
---
 core/util.c                 | 36 ++++++++++++++++++++++---
 corelib/lua_interface.c     |  2 +-
 handlers/delta_handler.c    |  2 +-
 handlers/diskpart_handler.c | 18 ++++++++-----
 include/util.h              |  3 ++-
 parser/parse_external.c     |  2 +-
 parser/parser.c             |  2 +-
 suricatta/common.c          |  2 +-
 suricatta/server_general.c  |  2 +-
 suricatta/server_hawkbit.c  |  2 +-
 test/Makefile               |  1 +
 test/test_util.c            | 52 +++++++++++++++++++++++++++++++++++++
 12 files changed, 106 insertions(+), 18 deletions(-)
 create mode 100644 test/test_util.c

Comments

Stefano Babic March 18, 2022, 8:14 p.m. UTC | #1 
On 18.03.22 16:13, Kyle Russell wrote:
> When passing in a partition size in units of sectors, fdisk expects you to
> enable explicit sizing on the partition; otherwise, fdisk assumes you want
> the partition aligned to the nearest grain (typically 1MB), which may
> corrupt a partition table containing already explicitly sized partitions.
> 
> This matches the behavior of other libfdisk clients in util-linux (see
> excerpt from libfdisk's parse_line_nameval() below):
> 
> libfdisk/src/script.c:
>          if (pow)        /* specified as <num><suffix> */
>                  num /= dp->cxt->sector_size;
>          else            /* specified as number of sectors */
>                  fdisk_partition_size_explicit(pa, 1);
> 
> To achieve this, we allow ustrtoull() to return a char* to the suffix
> of the converted string, if one exists.  If strtoull() points to a different
> suffix in the string than ustrtoull(), we assume ustrtoull() consumed a
> user provided size-type delimiter.  If no size-type delimiter match is found,
> explicit sizing is not enabled and fdisk aligns the partition to the nearest
> grain.
> 
> Signed-off-by: Kyle Russell <bkylerussell@gmail.com>
> ---

Applied to -master, thanks !

Best regards,
Stefano Babic
diff mbox series

Patch

diff --git a/core/util.c b/core/util.c
index 4c8c034..d9df906 100644
--- a/core/util.c
+++ b/core/util.c
@@ -743,7 +743,27 @@  void free_string_array(char **nodes)
 	free(nodes);
 }
 
-unsigned long long ustrtoull(const char *cp, unsigned int base)
+/*
+ * Determines if ustrtoull() consumes a size-type delimiter
+ * from the provided string.
+ */
+int size_delimiter_match(const char *size)
+{
+	char *suffix = NULL, *usuffix = NULL;
+	strtoull(size, &suffix, 10);
+	ustrtoull(size, &usuffix, 10);
+	return suffix != usuffix;
+}
+
+/*
+ * Like strtoull(), but automatically scales the conversion
+ * result by size-type units, and only returns a pointer to
+ * the size unit in the string if requested by the caller.
+ *
+ * Sets errno to ERANGE if strtoull() found no digits or
+ * encountered an overflow, and returns 0 in both cases.
+ */
+unsigned long long ustrtoull(const char *cp, char **endptr, unsigned int base)
 {
 	errno = 0;
 	char *endp = NULL;
@@ -756,7 +776,8 @@  unsigned long long ustrtoull(const char *cp, unsigned int base)
 
 	if (cp == endp || (result == ULLONG_MAX && errno == ERANGE)) {
 		errno = ERANGE;
-		return 0;
+		result = 0;
+		goto out;
 	}
 
 	switch (*endp) {
@@ -774,8 +795,15 @@  unsigned long long ustrtoull(const char *cp, unsigned int base)
 				endp += 3;
 			else
 				endp += 2;
+		} else {
+			endp += 1;
 		}
 	}
+
+out:
+	if (endptr)
+		*endptr = endp;
+
 	return result;
 }
 
@@ -1196,7 +1224,7 @@  long long get_output_size(struct img_type *img, bool strict)
 			return -ENOENT;
 		}
 
-		bytes = ustrtoull(output_size_str, 0);
+		bytes = ustrtoull(output_size_str, NULL, 0);
 		if (errno || bytes <= 0) {
 			ERROR("decompressed-size argument %s: ustrtoull failed",
 			      output_size_str);
@@ -1214,7 +1242,7 @@  long long get_output_size(struct img_type *img, bool strict)
 			return -ENOENT;
 		}
 
-		bytes = ustrtoull(output_size_str, 0);
+		bytes = ustrtoull(output_size_str, NULL, 0);
 		if (errno || bytes <= 0) {
 			ERROR("decrypted-size argument %s: ustrtoull failed",
 			      output_size_str);
diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
index c15592f..52b6605 100644
--- a/corelib/lua_interface.c
+++ b/corelib/lua_interface.c
@@ -319,7 +319,7 @@  static void lua_string_to_img(struct img_type *img, const char *key,
 		strncpy(seek_str, value,
 			sizeof(seek_str));
 		/* convert the offset handling multiplicative suffixes */
-		img->seek = ustrtoull(seek_str, 0);
+		img->seek = ustrtoull(seek_str, NULL, 0);
 		if (errno){
 			ERROR("offset argument: ustrtoull failed");
 		}
diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
index 3132f2e..f8d330c 100644
--- a/handlers/delta_handler.c
+++ b/handlers/delta_handler.c
@@ -329,7 +329,7 @@  static int delta_retrieve_attributes(struct img_type *img, struct hnd_priv *priv
 		if (!strcmp(srcsize, "detect"))
 			priv->detectsrcsize = true;
 		else
-			priv->srcsize = ustrtoull(srcsize, 10);
+			priv->srcsize = ustrtoull(srcsize, NULL, 10);
 	}
 
 	char *zckloglevel = dict_get_value(&img->properties, "zckloglevel");
diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
index fc9b169..0af4e60 100644
--- a/handlers/diskpart_handler.c
+++ b/handlers/diskpart_handler.c
@@ -74,6 +74,7 @@  struct partition_data {
 	char fstype[SWUPDATE_GENERAL_STRING_SIZE];
 	char dostype[SWUPDATE_GENERAL_STRING_SIZE];
 	char partuuid[UUID_STR_LEN];
+	int explicit_size;
 	LIST_ENTRY(partition_data) next;
 };
 LIST_HEAD(listparts, partition_data);
@@ -305,10 +306,13 @@  static int diskpart_set_partition(struct fdisk_partition *pa,
 		ret |= -EINVAL;
 	if (strlen(part->name))
 	      ret |= fdisk_partition_set_name(pa, part->name);
-	if (part->size != LIBFDISK_INIT_UNDEF(part->size))
+	if (part->size != LIBFDISK_INIT_UNDEF(part->size)) {
 	      ret |= fdisk_partition_set_size(pa, part->size / sector_size);
-	else
+	      if (part->explicit_size)
+			ret |= fdisk_partition_size_explicit(pa, part->explicit_size);
+	} else {
 		ret |= fdisk_partition_end_follow_default(pa, 1);
+	}
 
 	if (parttype)
 		ret |= fdisk_partition_set_type(pa, parttype);
@@ -543,7 +547,7 @@  static int diskpart_fill_table(struct fdisk_context *cxt, struct diskpart_table
 				parttype = fdisk_label_get_parttype_from_string(lb, GPT_DEFAULT_ENTRY_TYPE);
 			}
 		} else {
-			parttype = fdisk_label_get_parttype_from_code(lb, ustrtoull(part->type, 16));
+			parttype = fdisk_label_get_parttype_from_code(lb, ustrtoull(part->type, NULL, 16));
 		}
 		ret = diskpart_set_partition(newpa, part, sector_size, parttype, oldtb->parent);
 		if (ret) {
@@ -578,7 +582,7 @@  static int diskpart_fill_table(struct fdisk_context *cxt, struct diskpart_table
 
 				newpa = fdisk_new_partition();
 
-				parttype = fdisk_label_get_parttype_from_code(lb, ustrtoull(part->dostype, 16));
+				parttype = fdisk_label_get_parttype_from_code(lb, ustrtoull(part->dostype, NULL, 16));
 				if (!parttype) {
 					ERROR("I cannot add hybrid partition %zu(%s) invalid dostype: %s",
 						part->partno, part->name, part->dostype);
@@ -854,10 +858,12 @@  static int diskpart(struct img_type *img,
 					equal++;
 					switch (i) {
 					case PART_SIZE:
-						part->size = ustrtoull(equal, 10);
+						part->size = ustrtoull(equal, NULL, 10);
+						if (!size_delimiter_match(equal))
+							part->explicit_size = 1;
 						break;
 					case PART_START:
-						part->start = ustrtoull(equal, 10);
+						part->start = ustrtoull(equal, NULL, 10);
 						break;
 					case PART_TYPE:
 						strncpy(part->type, equal, sizeof(part->type));
diff --git a/include/util.h b/include/util.h
index 950046d..270978d 100644
--- a/include/util.h
+++ b/include/util.h
@@ -228,7 +228,8 @@  void set_version_range(const char *minversion,
 			const char *maxversion,
 			const char *current);
 
-unsigned long long ustrtoull(const char *cp, unsigned int base);
+int size_delimiter_match(const char *size);
+unsigned long long ustrtoull(const char *cp, char **endptr, unsigned int base);
 
 const char* get_tmpdir(void);
 const char* get_tmpdirscripts(void);
diff --git a/parser/parse_external.c b/parser/parse_external.c
index 6240c46..c6ab4fe 100644
--- a/parser/parse_external.c
+++ b/parser/parse_external.c
@@ -69,7 +69,7 @@  static void sw_append_stream(struct img_type *img, const char *key,
 		strlcpy(seek_str, value,
 			sizeof(seek_str));
 		/* convert the offset handling multiplicative suffixes */
-		img->seek = ustrtoull(seek_str, 0);
+		img->seek = ustrtoull(seek_str, NULL, 0);
 		if (errno){
 			ERROR("offset argument: ustrtoull failed");
 		}
diff --git a/parser/parser.c b/parser/parser.c
index 85241f4..eebd581 100644
--- a/parser/parser.c
+++ b/parser/parser.c
@@ -406,7 +406,7 @@  static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
 	get_hash_value(p, elem, image->sha256);
 
 	/* convert the offset handling multiplicative suffixes */
-	image->seek = ustrtoull(seek_str, 0);
+	image->seek = ustrtoull(seek_str, NULL, 0);
 	if (errno){
 		ERROR("offset argument: ustrtoull failed");
 		return -1;
diff --git a/suricatta/common.c b/suricatta/common.c
index 28c5e94..c47fa5c 100644
--- a/suricatta/common.c
+++ b/suricatta/common.c
@@ -25,7 +25,7 @@  void suricatta_channel_settings(void *elem, channel_data_t *chan)
 
 	GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "max-download-speed", tmp);
 	if (strlen(tmp))
-		chan->max_download_speed = (unsigned int)ustrtoull(tmp, 10);
+		chan->max_download_speed = (unsigned int)ustrtoull(tmp, NULL, 10);
 
 	GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "retrywait", tmp);
 	if (strlen(tmp))
diff --git a/suricatta/server_general.c b/suricatta/server_general.c
index e674441..47f9078 100644
--- a/suricatta/server_general.c
+++ b/suricatta/server_general.c
@@ -669,7 +669,7 @@  server_op_res_t server_start(char *fname, int argc, char *argv[])
 			break;
 		case 'n':
 			channel_data_defaults.max_download_speed =
-				(unsigned int)ustrtoull(optarg, 10);
+				(unsigned int)ustrtoull(optarg, NULL, 10);
 			break;
 
 		case '?':
diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
index 68ce32e..5a456e1 100644
--- a/suricatta/server_hawkbit.c
+++ b/suricatta/server_hawkbit.c
@@ -1848,7 +1848,7 @@  server_op_res_t server_start(char *fname, int argc, char *argv[])
 			break;
 		case 'n':
 			channel_data_defaults.max_download_speed =
-				(unsigned int)ustrtoull(optarg, 10);
+				(unsigned int)ustrtoull(optarg, NULL, 10);
 			break;
 		/* Ignore not recognized options, they can be already parsed by the caller */
 		case '?':
diff --git a/test/Makefile b/test/Makefile
index a9b9f71..fb89e8f 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -26,6 +26,7 @@  tests-$(CONFIG_SIGNED_IMAGES) += test_verify
 endif
 tests-$(CONFIG_SURICATTA_HAWKBIT) += test_json
 tests-$(CONFIG_SURICATTA_HAWKBIT) += test_server_hawkbit
+tests-y += test_util
 
 ccflags-y += -I$(src)/../
 
diff --git a/test/test_util.c b/test/test_util.c
new file mode 100644
index 0000000..d6f2c0a
--- /dev/null
+++ b/test/test_util.c
@@ -0,0 +1,52 @@ 
+// SPDX-FileCopyrightText: 2022 Kyle Russell <bkylerussell@gmail.com>
+//
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <errno.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include "util.h"
+
+static int util_setup(void **state)
+{
+	(void)state;
+	return 0;
+}
+
+static int util_teardown(void **state)
+{
+	(void)state;
+	return 0;
+}
+
+static void test_util_size_delimiter_match(void **state)
+{
+	(void)state;
+	assert_int_equal(size_delimiter_match("1024G, some fancy things"), 1);
+	assert_int_equal(size_delimiter_match("2048KiB"), 1);
+	assert_int_equal(size_delimiter_match("1073741824"), 0);
+}
+
+static void test_util_ustrtoull(void **state)
+{
+	(void)state;
+	char *suffix = NULL;
+	uint64_t size = ustrtoull("1024M, some fancy things", &suffix, 10);
+	assert_int_equal(errno, 0);
+	assert_int_equal(size, 1073741824);
+	assert_string_equal(suffix, ", some fancy things");
+}
+
+int main(void)
+{
+	int error_count = 0;
+	const struct CMUnitTest util_tests[] = {
+	    cmocka_unit_test(test_util_ustrtoull),
+	    cmocka_unit_test(test_util_size_delimiter_match)
+	};
+	error_count += cmocka_run_group_tests_name("util", util_tests,
+						   util_setup, util_teardown);
+	return error_count;
+}