diff mbox series

fastboot: mmc: Fix dest size in strlcat()

Message ID 20210930134740.2451149-1-ariel.dalessandro@collabora.com
State Rejected
Delegated to: Jaehoon Chung
Headers show
Series fastboot: mmc: Fix dest size in strlcat() | expand

Commit Message

Ariel D'Alessandro Sept. 30, 2021, 1:47 p.m. UTC
strlcat() is truncating the destination string to the size of the source
string. Fix size argument in strlcat() to match the size of destination
buffer.

Bug was introduced in the following commit:

commit 69a752983171c2983fc1f29c7cfa1844f41e5d8b
Author: Sean Anderson <seanga2@gmail.com>

    fastboot: Fix possible buffer overrun

Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
---
 drivers/fastboot/fb_mmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ariel D'Alessandro Sept. 30, 2021, 3:17 p.m. UTC | #1
Hi Sean,

On 9/30/21 12:15 PM, Sean Anderson wrote:
> 
> 
> 
> On 9/30/21 9:47 AM, Ariel D'Alessandro wrote:
>> strlcat() is truncating the destination string to the size of the source
>> string. Fix size argument in strlcat() to match the size of destination
>> buffer.
>>
>> Bug was introduced in the following commit:
>>
>> commit 69a752983171c2983fc1f29c7cfa1844f41e5d8b
>> Author: Sean Anderson <seanga2@gmail.com>
>>
>>      fastboot: Fix possible buffer overrun
>>
>> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
>> ---
>>   drivers/fastboot/fb_mmc.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
>> index cbb3f7b1dea..4c54f0c3893 100644
>> --- a/drivers/fastboot/fb_mmc.c
>> +++ b/drivers/fastboot/fb_mmc.c
>> @@ -40,7 +40,7 @@ static int raw_part_get_info_by_name(struct blk_desc
>> *dev_desc,
>>
>>       /* check for raw partition descriptor */
>>       strcpy(env_desc_name, "fastboot_raw_partition_");
>> -    strlcat(env_desc_name, name, PART_NAME_LEN);
>> +    strlcat(env_desc_name, name, ARRAY_SIZE(env_desc_name));
>>       raw_part_desc = strdup(env_get(env_desc_name));
>>       if (raw_part_desc == NULL)
>>           return -ENODEV;
>> @@ -114,7 +114,7 @@ static int part_get_info_by_name_or_alias(struct
>> blk_desc **dev_desc,
>>
>>           /* check for alias */
>>           strcpy(env_alias_name, "fastboot_partition_alias_");
>> -        strlcat(env_alias_name, name, PART_NAME_LEN);
>> +        strlcat(env_alias_name, name, ARRAY_SIZE(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,
>>
> 
> This looks like a duplicate of [1]. I think the only difference is
> ARRAY_SIZE vs sizeof, but they are equivalent since sizeof(char) == 1.

Ah, you're right. Sorry I missed that patch.

> 
> --Sean
> 
> [1]
> http://patchwork.ozlabs.org/project/uboot/patch/20210730122354.6281-1-matthias.schiffer@ew.tq-group.com/
>
diff mbox series

Patch

diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
index cbb3f7b1dea..4c54f0c3893 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -40,7 +40,7 @@  static int raw_part_get_info_by_name(struct blk_desc *dev_desc,
 
 	/* check for raw partition descriptor */
 	strcpy(env_desc_name, "fastboot_raw_partition_");
-	strlcat(env_desc_name, name, PART_NAME_LEN);
+	strlcat(env_desc_name, name, ARRAY_SIZE(env_desc_name));
 	raw_part_desc = strdup(env_get(env_desc_name));
 	if (raw_part_desc == NULL)
 		return -ENODEV;
@@ -114,7 +114,7 @@  static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc,
 
 		/* check for alias */
 		strcpy(env_alias_name, "fastboot_partition_alias_");
-		strlcat(env_alias_name, name, PART_NAME_LEN);
+		strlcat(env_alias_name, name, ARRAY_SIZE(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,