[PATCH] fastboot: only look up real partition names when no alias exists

Having U-Boot look up the passed partition name even though an alias exists is unexpected, leading to warning messages (when the alias name doesn't exist as a real partition name) or the use of the wrong partition.
Change part_get_info_by_name_or_alias() to consider real partitions names only if no alias of the same name exists, allowing to use aliases to override the configuration for existing partition names.
Also change one use of strcpy() to strlcpy().
Signed-off-by: Matthias Schiffer matthias.schiffer@ew.tq-group.com --- drivers/fastboot/fb_mmc.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index 2738dc836e..fb7791d9da 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -104,23 +104,18 @@ static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, const char *name, struct disk_partition *info) { - int ret; - - ret = do_get_part_info(dev_desc, name, info); - if (ret < 0) { - /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */ - char env_alias_name[25 + PART_NAME_LEN + 1]; - char *aliased_part_name; - - /* check for alias */ - strcpy(env_alias_name, "fastboot_partition_alias_"); - strlcat(env_alias_name, name, sizeof(env_alias_name)); - aliased_part_name = env_get(env_alias_name); - if (aliased_part_name != NULL) - ret = do_get_part_info(dev_desc, aliased_part_name, - info); - } - return ret; + /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */ + char env_alias_name[25 + PART_NAME_LEN + 1]; + char *aliased_part_name; + + /* check for alias */ + strlcpy(env_alias_name, "fastboot_partition_alias_", sizeof(env_alias_name)); + strlcat(env_alias_name, name, sizeof(env_alias_name)); + aliased_part_name = env_get(env_alias_name); + if (aliased_part_name) + name = aliased_part_name; + + return do_get_part_info(dev_desc, name, info); }
/**

Hi Matthias,
On 12/16/21 5:26 AM, Matthias Schiffer wrote:
Having U-Boot look up the passed partition name even though an alias exists is unexpected, leading to warning messages (when the alias name doesn't exist as a real partition name) or the use of the wrong partition.
Change part_get_info_by_name_or_alias() to consider real partitions names only if no alias of the same name exists, allowing to use aliases to override the configuration for existing partition names.
Much saner IMO.
I think the correct move in the long term is to add a "quiet" parameter to do_get_part_info (and all its helpers). This is OK as an incremental improvement.
Also change one use of strcpy() to strlcpy().
Signed-off-by: Matthias Schiffer matthias.schiffer@ew.tq-group.com
drivers/fastboot/fb_mmc.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index 2738dc836e..fb7791d9da 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -104,23 +104,18 @@ static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, const char *name, struct disk_partition *info) {
- int ret;
- ret = do_get_part_info(dev_desc, name, info);
- if (ret < 0) {
/* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
char env_alias_name[25 + PART_NAME_LEN + 1];
char *aliased_part_name;
/* check for alias */
strcpy(env_alias_name, "fastboot_partition_alias_");
strlcat(env_alias_name, name, sizeof(env_alias_name));
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name != NULL)
ret = do_get_part_info(dev_desc, aliased_part_name,
info);
- }
- return ret;
/* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
char env_alias_name[25 + PART_NAME_LEN + 1];
char *aliased_part_name;
/* check for alias */
strlcpy(env_alias_name, "fastboot_partition_alias_", sizeof(env_alias_name));
strlcat(env_alias_name, name, sizeof(env_alias_name));
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name)
name = aliased_part_name;
return do_get_part_info(dev_desc, name, info); }
/**
Reviewed-by: Sean Anderson sean.anderson@seco.com
--Sean

On Fri, 2021-12-17 at 18:20 -0500, Sean Anderson wrote:
Hi Matthias,
On 12/16/21 5:26 AM, Matthias Schiffer wrote:
Having U-Boot look up the passed partition name even though an alias exists is unexpected, leading to warning messages (when the alias name doesn't exist as a real partition name) or the use of the wrong partition.
Change part_get_info_by_name_or_alias() to consider real partitions names only if no alias of the same name exists, allowing to use aliases to override the configuration for existing partition names.
Much saner IMO.
I think the correct move in the long term is to add a "quiet" parameter to do_get_part_info (and all its helpers). This is OK as an incremental improvement.
Also change one use of strcpy() to strlcpy().
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com
drivers/fastboot/fb_mmc.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index 2738dc836e..fb7791d9da 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -104,23 +104,18 @@ static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, const char *name, struct disk_partition *info) {
- int ret;
- ret = do_get_part_info(dev_desc, name, info);
- if (ret < 0) {
/* strlen("fastboot_partition_alias_") + PART_NAME_LEN
- 1 */
char env_alias_name[25 + PART_NAME_LEN + 1];
char *aliased_part_name;
/* check for alias */
strcpy(env_alias_name, "fastboot_partition_alias_");
strlcat(env_alias_name, name, sizeof(env_alias_name));
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name != NULL)
ret = do_get_part_info(dev_desc,
aliased_part_name,
info);
- }
- return ret;
- /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
- char env_alias_name[25 + PART_NAME_LEN + 1];
- char *aliased_part_name;
- /* check for alias */
- strlcpy(env_alias_name, "fastboot_partition_alias_",
sizeof(env_alias_name));
strlcat(env_alias_name, name, sizeof(env_alias_name));
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name)
name = aliased_part_name;
return do_get_part_info(dev_desc, name, info); }
/**
Reviewed-by: Sean Anderson sean.anderson@seco.com
--Sean
Can we get this committed?
Kind regards, Matthias

On 1/26/22 4:54 AM, Matthias Schiffer wrote:
On Fri, 2021-12-17 at 18:20 -0500, Sean Anderson wrote:
Hi Matthias,
On 12/16/21 5:26 AM, Matthias Schiffer wrote:
Having U-Boot look up the passed partition name even though an alias exists is unexpected, leading to warning messages (when the alias name doesn't exist as a real partition name) or the use of the wrong partition.
Change part_get_info_by_name_or_alias() to consider real partitions names only if no alias of the same name exists, allowing to use aliases to override the configuration for existing partition names.
Much saner IMO.
I think the correct move in the long term is to add a "quiet" parameter to do_get_part_info (and all its helpers). This is OK as an incremental improvement.
Also change one use of strcpy() to strlcpy().
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com
drivers/fastboot/fb_mmc.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index 2738dc836e..fb7791d9da 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -104,23 +104,18 @@ static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, const char *name, struct disk_partition *info) {
- int ret;
- ret = do_get_part_info(dev_desc, name, info);
- if (ret < 0) {
/* strlen("fastboot_partition_alias_") + PART_NAME_LEN
- 1 */
char env_alias_name[25 + PART_NAME_LEN + 1];
char *aliased_part_name;
/* check for alias */
strcpy(env_alias_name, "fastboot_partition_alias_");
strlcat(env_alias_name, name, sizeof(env_alias_name));
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name != NULL)
ret = do_get_part_info(dev_desc,
aliased_part_name,
info);
- }
- return ret;
- /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
- char env_alias_name[25 + PART_NAME_LEN + 1];
- char *aliased_part_name;
- /* check for alias */
- strlcpy(env_alias_name, "fastboot_partition_alias_",
sizeof(env_alias_name));
strlcat(env_alias_name, name, sizeof(env_alias_name));
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name)
name = aliased_part_name;
return do_get_part_info(dev_desc, name, info); }
/**
Reviewed-by: Sean Anderson sean.anderson@seco.com
--Sean
Can we get this committed?
+CC Tom

On Thu, Jan 27, 2022 at 11:22:12AM -0500, Sean Anderson wrote:
On 1/26/22 4:54 AM, Matthias Schiffer wrote:
On Fri, 2021-12-17 at 18:20 -0500, Sean Anderson wrote:
Hi Matthias,
On 12/16/21 5:26 AM, Matthias Schiffer wrote:
Having U-Boot look up the passed partition name even though an alias exists is unexpected, leading to warning messages (when the alias name doesn't exist as a real partition name) or the use of the wrong partition.
Change part_get_info_by_name_or_alias() to consider real partitions names only if no alias of the same name exists, allowing to use aliases to override the configuration for existing partition names.
Much saner IMO.
I think the correct move in the long term is to add a "quiet" parameter to do_get_part_info (and all its helpers). This is OK as an incremental improvement.
Also change one use of strcpy() to strlcpy().
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com
drivers/fastboot/fb_mmc.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index 2738dc836e..fb7791d9da 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -104,23 +104,18 @@ static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, const char *name, struct disk_partition *info) {
- int ret;
- ret = do_get_part_info(dev_desc, name, info);
- if (ret < 0) {
/* strlen("fastboot_partition_alias_") + PART_NAME_LEN
- 1 */
char env_alias_name[25 + PART_NAME_LEN + 1];
char *aliased_part_name;
/* check for alias */
strcpy(env_alias_name, "fastboot_partition_alias_");
strlcat(env_alias_name, name, sizeof(env_alias_name));
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name != NULL)
ret = do_get_part_info(dev_desc,
aliased_part_name,
info);
- }
- return ret;
- /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */
- char env_alias_name[25 + PART_NAME_LEN + 1];
- char *aliased_part_name;
- /* check for alias */
- strlcpy(env_alias_name, "fastboot_partition_alias_",
sizeof(env_alias_name));
strlcat(env_alias_name, name, sizeof(env_alias_name));
aliased_part_name = env_get(env_alias_name);
if (aliased_part_name)
name = aliased_part_name;
return do_get_part_info(dev_desc, name, info); }
/**
Reviewed-by: Sean Anderson sean.anderson@seco.com
--Sean
Can we get this committed?
+CC Tom
There's a number of fastboot things and I guess I need to see what I can try and review and grab or reassign.

On Thu, Dec 16, 2021 at 11:26:38AM +0100, Matthias Schiffer wrote:
Having U-Boot look up the passed partition name even though an alias exists is unexpected, leading to warning messages (when the alias name doesn't exist as a real partition name) or the use of the wrong partition.
Change part_get_info_by_name_or_alias() to consider real partitions names only if no alias of the same name exists, allowing to use aliases to override the configuration for existing partition names.
Also change one use of strcpy() to strlcpy().
Signed-off-by: Matthias Schiffer matthias.schiffer@ew.tq-group.com Reviewed-by: Sean Anderson sean.anderson@seco.com
Applied to u-boot/master, thanks!
participants (3)
-
Matthias Schiffer
-
Sean Anderson
-
Tom Rini