[PATCH 0/6] fastboot: fix handling of non-existent partitions

[PATCH 0/6] fastboot: fix handling of non-existent partitions

[Ahmad Fatoum]

To allow flashing an image with a partition table followed by an image
for a specific partition, the fastboot implementation was changed to
evaluate partitions on demand.

This introduced issues around handling of non-existent files, which
should be resolved by this series.

I think it's fine for the series to go through next, but would like at
least the first patch to go into master.

Ahmad Fatoum (6):
  fastboot: terminate request for non-file-backed partition properly
  fastboot: introduce fastboot_tx_print_var() helper
  fastboot: don't populate partition variables unconditionally
  fastboot: only populate variables explicitly asked for
  fastboot: factor out fb_file_getsize
  fastboot: check for file existence before flashing/erasing

 common/fastboot.c | 138 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 98 insertions(+), 40 deletions(-)

-- 
2.39.5

[PATCH 1/6] fastboot: terminate request for non-file-backed partition properly

[Ahmad Fatoum]

If addition of fastboot variables fail due to a specific partition, it's
not enough to log a warning at the barebox side, but we need to return
an error to the client too, otherwise it will hang indefinitely.

Fixes: 6a191155be4e ("fastboot: evaluate fastboot partitions when used")
Reported-by: Uwe Kleine-König <uwe@kleine-koenig.org>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/fastboot.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index 34ac00bb0ecf..5a43bfb7e596 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -298,8 +298,12 @@ static void cb_getvar(struct fastboot *fb, const char *cmd)
 
 		ret = fastboot_add_partition_variables(fb, &partition_list, fentry);
 		if (ret) {
-			pr_warn("Failed to add partition variables: %pe", ERR_PTR(ret));
-			return;
+			char *msg = xasprintf("%s: Failed to add '%s' partition variables: %pe",
+					      fentry->name, fentry->filename, ERR_PTR(ret));
+			pr_warn("%s\n", msg);
+			fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, "%s", msg);
+			free(msg);
+			goto out;
 		}
 	}
 
-- 
2.39.5

[PATCH 2/6] fastboot: introduce fastboot_tx_print_var() helper

[Ahmad Fatoum]

We have 4 loops that we could avoid duplicating by adding a helper that
iterates and does the comparison inside. This increases code size a bit,
but will come in handy next commit, when we refactor the code to avoid
unnecessary computation.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/fastboot.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index 5a43bfb7e596..3155ff2af92c 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -287,9 +287,27 @@ static void cb_reboot(struct fastboot *fb, const char *cmd)
 	restart_machine(0);
 }
 
-static void cb_getvar(struct fastboot *fb, const char *cmd)
+static bool fastboot_tx_print_var(struct fastboot *fb, struct list_head *vars,
+				  const char *varname)
 {
 	struct fb_variable *var;
+
+	list_for_each_entry(var, vars, list) {
+		if (!varname) {
+			fastboot_tx_print(fb, FASTBOOT_MSG_INFO, "%s: %s",
+					  var->name, var->value);
+		} else if (!strcmp(varname, var->name)) {
+			fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "%s",
+					  var->value);
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static void cb_getvar(struct fastboot *fb, const char *cmd)
+{
 	LIST_HEAD(partition_list);
 	struct file_list_entry *fentry;
 
@@ -310,31 +328,18 @@ static void cb_getvar(struct fastboot *fb, const char *cmd)
 	pr_debug("getvar: \"%s\"\n", cmd);
 
 	if (!strcmp(cmd, "all")) {
-		list_for_each_entry(var, &fb->variables, list)
-			fastboot_tx_print(fb, FASTBOOT_MSG_INFO, "%s: %s",
-					  var->name, var->value);
-
-		list_for_each_entry(var, &partition_list, list)
-			fastboot_tx_print(fb, FASTBOOT_MSG_INFO, "%s: %s",
-					  var->name, var->value);
+		fastboot_tx_print_var(fb, &fb->variables, NULL);
+		fastboot_tx_print_var(fb, &partition_list, NULL);
 
 		fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "");
 		goto out;
 	}
 
-	list_for_each_entry(var, &fb->variables, list) {
-		if (!strcmp(cmd, var->name)) {
-			fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, var->value);
-			goto out;
-		}
-	}
+	if (fastboot_tx_print_var(fb, &fb->variables, cmd))
+		goto out;
 
-	list_for_each_entry(var, &partition_list, list) {
-		if (!strcmp(cmd, var->name)) {
-			fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, var->value);
-			goto out;
-		}
-	}
+	if (fastboot_tx_print_var(fb, &partition_list, cmd))
+		goto out;
 
 	fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "");
 out:
-- 
2.39.5

[PATCH 3/6] fastboot: don't populate partition variables unconditionally

[Ahmad Fatoum]

If the client asks for max-download-size, there is no need to enumerate
all partitions, so reorder the code accordingly.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/fastboot.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index 3155ff2af92c..231f49c1a6c9 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -310,6 +310,16 @@ static void cb_getvar(struct fastboot *fb, const char *cmd)
 {
 	LIST_HEAD(partition_list);
 	struct file_list_entry *fentry;
+	bool all;
+
+	pr_debug("getvar: \"%s\"\n", cmd);
+
+	all = !strcmp(cmd, "all");
+	if (all)
+		cmd = NULL;
+
+	if (fastboot_tx_print_var(fb, &fb->variables, cmd))
+		goto out;
 
 	file_list_for_each_entry(fb->files, fentry) {
 		int ret;
@@ -325,19 +335,6 @@ static void cb_getvar(struct fastboot *fb, const char *cmd)
 		}
 	}
 
-	pr_debug("getvar: \"%s\"\n", cmd);
-
-	if (!strcmp(cmd, "all")) {
-		fastboot_tx_print_var(fb, &fb->variables, NULL);
-		fastboot_tx_print_var(fb, &partition_list, NULL);
-
-		fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "");
-		goto out;
-	}
-
-	if (fastboot_tx_print_var(fb, &fb->variables, cmd))
-		goto out;
-
 	if (fastboot_tx_print_var(fb, &partition_list, cmd))
 		goto out;
 
-- 
2.39.5

[PATCH 4/6] fastboot: only populate variables explicitly asked for

[Ahmad Fatoum]

To allow flashing an image with a partition table followed by an image
for a specific partition, the fastboot implementation was changed to
evaluate partitions on demand.

As there are partition-specific variables and the magic "all" variable,
which lists all variables, all partitions are evaluated in getvar
unconditionally, when only a variable for a specific partition is needed.

When one partition doesn't exist and is not marked as optional, this
leads to an error, even if something innocuous as the general
max-download-size is requested.

Change this, so only partitions that were explicitly asked for have their
variables populated on demand, unless "all" variables were requested.

Fixes: 6a191155be4e ("fastboot: evaluate fastboot partitions when used")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/fastboot.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/common/fastboot.c b/common/fastboot.c
index 231f49c1a6c9..529d9690db79 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -310,10 +310,15 @@ static void cb_getvar(struct fastboot *fb, const char *cmd)
 {
 	LIST_HEAD(partition_list);
 	struct file_list_entry *fentry;
+	const char *partition;
 	bool all;
 
 	pr_debug("getvar: \"%s\"\n", cmd);
 
+	partition = strchr(cmd, ':');
+	if (partition)
+		partition++;
+
 	all = !strcmp(cmd, "all");
 	if (all)
 		cmd = NULL;
@@ -321,9 +326,15 @@ static void cb_getvar(struct fastboot *fb, const char *cmd)
 	if (fastboot_tx_print_var(fb, &fb->variables, cmd))
 		goto out;
 
+	if (!all && !partition)
+		goto skip_partitions;
+
 	file_list_for_each_entry(fb->files, fentry) {
 		int ret;
 
+		if (!all && strcmp(partition, fentry->name))
+			continue;
+
 		ret = fastboot_add_partition_variables(fb, &partition_list, fentry);
 		if (ret) {
 			char *msg = xasprintf("%s: Failed to add '%s' partition variables: %pe",
@@ -338,6 +349,7 @@ static void cb_getvar(struct fastboot *fb, const char *cmd)
 	if (fastboot_tx_print_var(fb, &partition_list, cmd))
 		goto out;
 
+skip_partitions:
 	fastboot_tx_print(fb, FASTBOOT_MSG_OKAY, "");
 out:
 	fastboot_free_variables(&partition_list);
-- 
2.39.5

[PATCH 5/6] fastboot: factor out fb_file_getsize

[Ahmad Fatoum]

We'll add another user for stat on the fentry file name, so prepare for
that by factoring out the code into a helper.

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

diff --git a/common/fastboot.c b/common/fastboot.c
index 529d9690db79..e51a6b2027ec 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -80,15 +80,10 @@ static struct fb_variable *fb_addvar(struct fastboot *fb, struct list_head *list
 	return var;
 }
 
-static int fastboot_add_partition_variables(struct fastboot *fb, struct list_head *list,
-		struct file_list_entry *fentry)
+static loff_t fb_file_getsize(struct file_list_entry *fentry)
 {
 	struct stat s;
-	size_t size = 0;
-	int fd, ret;
-	struct mtd_info_user mtdinfo;
-	char *type = NULL;
-	struct fb_variable *var;
+	int ret;
 
 	ret = stat(fentry->filename, &s);
 	if (ret) {
@@ -96,7 +91,20 @@ static int fastboot_add_partition_variables(struct fastboot *fb, struct list_hea
 		ret = stat(fentry->filename, &s);
 	}
 
-	if (ret) {
+	return ret ? ret : s.st_size;
+}
+
+static int fastboot_add_partition_variables(struct fastboot *fb, struct list_head *list,
+		struct file_list_entry *fentry)
+{
+	loff_t size;
+	int fd, ret;
+	struct mtd_info_user mtdinfo;
+	char *type = NULL;
+	struct fb_variable *var;
+
+	size = fb_file_getsize(fentry);
+	if (size < 0) {
 		if (fentry->flags & FILE_LIST_FLAG_OPTIONAL) {
 			pr_info("skipping unavailable optional partition %s for fastboot gadget\n",
 				fentry->filename);
@@ -111,6 +119,7 @@ static int fastboot_add_partition_variables(struct fastboot *fb, struct list_hea
 			goto out;
 		}
 
+		ret = size;
 		goto out;
 	}
 
@@ -120,7 +129,6 @@ static int fastboot_add_partition_variables(struct fastboot *fb, struct list_hea
 		goto out;
 	}
 
-	size = s.st_size;
 
 	ret = ioctl(fd, MEMGETINFO, &mtdinfo);
 
-- 
2.39.5

[PATCH 6/6] fastboot: check for file existence before flashing/erasing

[Ahmad Fatoum]

We used to have an init time check for whether exported files exist or
not, but this went away and was replaced by a check in getvar.

If a client omits getvar and directly calls flash for a non-existent
file, barebox will create it automatically, thereby turning every
partition as if it had the create flag set.

Add an explicit check for whether a file exists into flash/erase
and proceed only if it does or the create flag is set.

The optional flag needs not to be checked here as it's only relevant
when enumerating available partitions.

Fixes: 6a191155be4e ("fastboot: evaluate fastboot partitions when used")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/fastboot.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index e51a6b2027ec..56bc4e82c4fe 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -94,6 +94,20 @@ static loff_t fb_file_getsize(struct file_list_entry *fentry)
 	return ret ? ret : s.st_size;
 }
 
+static int fb_file_available(struct file_list_entry *fentry)
+{
+	loff_t size;
+
+	size = fb_file_getsize(fentry);
+	if (size >= 0)
+		return 1;
+
+	if (fentry->flags & FILE_LIST_FLAG_CREATE)
+		return 0;
+
+	return size;
+}
+
 static int fastboot_add_partition_variables(struct fastboot *fb, struct list_head *list,
 		struct file_list_entry *fentry)
 {
@@ -682,13 +696,24 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
 		goto out;
 	}
 
-	/* Check if board-code registered a vendor-specific handler */
+	/* Check if board-code registered a vendor-specific handler
+	 * We intentionally do this before the fb_file_available check
+	 * to afford board core more flexibility.
+	 */
 	if (fb->cmd_flash) {
 		ret = fb->cmd_flash(fb, fentry, fb->tempname, fb->download_size);
 		if (ret != FASTBOOT_CMD_FALLTHROUGH)
 			goto out;
 	}
 
+	ret = fb_file_available(fentry);
+	if (ret < 0) {
+		fastboot_tx_print(fb, FASTBOOT_MSG_FAIL,
+				  "file %s doesn't exist", fentry->filename);
+		ret = -ENOENT;
+		goto out;
+	}
+
 	filename = fentry->filename;
 
 	if (filetype == filetype_android_sparse) {
@@ -801,6 +826,13 @@ static void cb_erase(struct fastboot *fb, const char *cmd)
 		return;
 	}
 
+	ret = fb_file_available(fentry);
+	if (ret < 0) {
+		fastboot_tx_print(fb, FASTBOOT_MSG_FAIL,
+				  "file %s doesn't exist", fentry->filename);
+		return;
+	}
+
 	fd = open(filename, O_RDWR);
 	if (fd < 0)
 		fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, strerror(-fd));
-- 
2.39.5