[U-Boot] [PATCH 1/2] fastboot: getvar_partition_{type, size}: Sanitize arguments
Correct usage of "fastboot getvar_partition_{type,size}" is:
host $> fastboot getvar partition-size:misc partition-size:misc: 0x0000000000000400 Finished. Total time: 0.214s
When the partition name is omitted, current behavior is:
host $> fastboot getvar partition-size getvar:partition-size FAILED (remote: partition not found) finished. total time: 0.005s host $> fastboot getvar partition-size: getvar:partition-size: FAILED (remote: partition not found) finished. total time: 0.006s host $> fastboot getvar partition-type getvar:partition-type FAILED (remote: partition not found) finished. total time: 0.005s host $> fastboot getvar partition-type: getvar:partition-type: FAILED (remote: partition not found) finished. total time: 0.006s
Tell the user the real cause of command failure:
host $> fastboot getvar partition-size getvar:partition-size FAILED (remote: missing partition name) Finished. Total time: 0.003s host $> fastboot getvar partition-size: getvar:partition-size: FAILED (remote: missing partition name) Finished. Total time: 0.003s host $> fastboot getvar partition-type getvar:partition-type FAILED (remote: missing partition name) Finished. Total time: 0.003s host $> fastboot getvar partition-type: getvar:partition-type: FAILED (remote: missing partition name) Finished. Total time: 0.003s
Fixes: f73a7df984a9 ("net: fastboot: Merge AOSP UDP fastboot") Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com --- drivers/fastboot/fb_getvar.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 4d264c985d7e..28e3e2fa1619 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -144,6 +144,11 @@ static void getvar_partition_type(char *part_name, char *response) struct blk_desc *dev_desc; disk_partition_t part_info;
+ if (!part_name || !strcmp(part_name, "")) { + fastboot_fail("missing partition name", response); + return; + } + r = fastboot_mmc_get_part_info(part_name, &dev_desc, &part_info, response); if (r >= 0) { @@ -162,6 +167,11 @@ static void getvar_partition_size(char *part_name, char *response) int r; size_t size;
+ if (!part_name || !strcmp(part_name, "")) { + fastboot_fail("missing partition name", response); + return; + } + #if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC) struct blk_desc *dev_desc; disk_partition_t part_info;
Only a handful of Android/fastboot partitions (e.g. /system, /vendor, /userdata) have filesystem:
host $> fastboot getvar partition-type:userdata partition-type:userdata: ext4 Finished. Total time: 0.013s
Most of them (/misc, /pstore, /vbmeta, /dtb{o}, /boot, etc) don't. And for the latter fastboot reports:
host $> fastboot getvar partition-type:misc getvar:partition-type:misc FAILED (remote: failed to set partition) Finished. Total time: 0.219s
Rather than creating pointless worries via error reporting, tell the users they are dealing with a 'raw' partition:
host $> fastboot getvar partition-type:misc partition-type:misc: raw Finished. Total time: 0.017s
Fixes: f73a7df984a9 ("net: fastboot: Merge AOSP UDP fastboot") Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com --- drivers/fastboot/fb_getvar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index 28e3e2fa1619..beadf7f98e5d 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -154,7 +154,7 @@ static void getvar_partition_type(char *part_name, char *response) if (r >= 0) { r = fs_set_blk_dev_with_part(dev_desc, r); if (r < 0) - fastboot_fail("failed to set partition", response); + fastboot_okay("raw", response); else fastboot_okay(fs_get_type_name(), response); }
Dear reviewers,
On Tue, Mar 19, 2019 at 11:57:06PM +0100, Eugeniu Rosca wrote:
fastboot_fail("failed to set partition", response);
fastboot_okay("raw", response);
While the issues fixed by the two patches are minor, I strongly believe they improve the user experience and I would very much appreciate any concerns/comments from you.
Many thanks, Eugeniu.
On Tue, Mar 19, 2019 at 11:57:06PM +0100, Eugeniu Rosca wrote: [..]
fastboot_fail("failed to set partition", response);
fastboot_okay("raw", response);
[..]
Checking recent upstream fastboot implementation [1], I can see [2] that returning success or failure on calling 'partition-type:<part-name>' makes an impact on the internal fastboot code paths, so unfortunately I have to NAK my own patch. One idea could be calling fastboot_fail() (not fastboot_okay as this patch proposed) with an updated error message. Please, skip the patch for now.
[1] https://android.googlesource.com/platform/system/core/+/eac1220fba2c [2] core (eac1220fba2c) git grep -C 1 partition-type fastboot/constants.h-#define FB_VAR_PARTITION_SIZE "partition-size" fastboot/constants.h:#define FB_VAR_PARTITION_TYPE "partition-type" fastboot/constants.h-#define FB_VAR_SLOT_SUCCESSFUL "slot-successful" -- fastboot/fastboot.cpp- fastboot/fastboot.cpp: if (fb->GetVar("partition-type:" + partition, &partition_type) != fastboot::SUCCESS) { fastboot/fastboot.cpp- errMsg = "Can't determine partition type.\n"; -- fastboot/fastboot.cpp- std::string partition_type; fastboot/fastboot.cpp: if (fb->GetVar("partition-type:" + partition, &partition_type) == fastboot::SUCCESS && fastboot/fastboot.cpp- fs_get_generator(partition_type) != nullptr) { -- fastboot/fastboot.cpp- std::string partition_type; fastboot/fastboot.cpp: if (fb->GetVar("partition-type:" + partition, &partition_type) != fastboot::SUCCESS) { fastboot/fastboot.cpp- continue; -- fastboot/fuzzy_fastboot/main.cpp- std::string partition_type; fastboot/fuzzy_fastboot/main.cpp: // getvar partition-type:super must fail for retrofit devices because the fastboot/fuzzy_fastboot/main.cpp- // partition does not exist. fastboot/fuzzy_fastboot/main.cpp: if (fb->GetVar("partition-type:super", &partition_type) == SUCCESS) { fastboot/fuzzy_fastboot/main.cpp- std::string is_logical; -- fastboot/fuzzy_fastboot/main.cpp- std::string resp; fastboot/fuzzy_fastboot/main.cpp: EXPECT_EQ(fb->GetVar("partition-type:" + part, &resp), SUCCESS); fastboot/fuzzy_fastboot/main.cpp: EXPECT_NE(allowed.find(resp), allowed.end()) << "getvar:partition-type:" + part << " was '" fastboot/fuzzy_fastboot/main.cpp- << resp << "' this is not a valid type"; -- fs_mgr/tests/adb-remount-test.sh- if [ -n "${scratch_paritition}" ]; then fs_mgr/tests/adb-remount-test.sh: fastboot_getvar partition-type:${scratch_partition} raw || fs_mgr/tests/adb-remount-test.sh- ( fastboot reboot && false) ||
Best regards, Eugeniu.
Superseded by https://patchwork.ozlabs.org/patch/1068195/ ("[U-Boot,v2] fastboot: Improve error reporting on 'getvar partition-{size, type}'")
participants (1)
-
Eugeniu Rosca