[v2] nvmem: layouts: u-boot-env: check earlier for ethaddr length

[PATCH v2] nvmem: layouts: u-boot-env: check earlier for ethaddr length

Posted by Tomasz Maciej Nowak 1 month, 1 week ago

From: Tomasz Maciej Nowak <tmn505@gmail.com>

Unfortunately the ethaddr value in U-Boot environment might be enclosed
in single/double quotes or be something completely different. This can
make it different than MAC_ADDR_STR_LEN, which results in EINVAL
returned by ethaddr post process. Move the check for length earlier,
to skip post processing, so nvmem could still present ethaddr value as
a string if the value doesn't match MAC_ADDR_STR_LEN.

Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com>
---

v2
Better clarification in commit message

v1
Link: https://lore.kernel.org/all/20260319171236.48325-1-tmn505@terefe.re/

 drivers/nvmem/layouts/u-boot-env.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c
index f27f387bb52a..dd3f541728bf 100644
--- a/drivers/nvmem/layouts/u-boot-env.c
+++ b/drivers/nvmem/layouts/u-boot-env.c
@@ -38,9 +38,6 @@ static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, i
 {
 	u8 mac[ETH_ALEN];
 
-	if (bytes != MAC_ADDR_STR_LEN)
-		return -EINVAL;
-
 	if (!mac_pton(buf, mac))
 		return -EINVAL;
 
@@ -75,7 +72,7 @@ static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem
 		info.offset = data_offset + value - data;
 		info.bytes = strlen(value);
 		info.np = of_get_child_by_name(dev->of_node, info.name);
-		if (!strcmp(var, "ethaddr")) {
+		if ((!strcmp(var, "ethaddr")) && (info.bytes == MAC_ADDR_STR_LEN)) {
 			info.raw_len = strlen(value);
 			info.bytes = ETH_ALEN;
 			info.read_post_process = u_boot_env_read_post_process_ethaddr;
-- 
2.54.0

Re: [PATCH v2] nvmem: layouts: u-boot-env: check earlier for ethaddr length

Posted by Srinivas Kandagatla 3 weeks, 4 days ago


On 5/4/26 6:19 PM, Tomasz Maciej Nowak wrote:
> From: Tomasz Maciej Nowak <tmn505@gmail.com>
> 
> Unfortunately the ethaddr value in U-Boot environment might be enclosed
> in single/double quotes or be something completely different. This can
> make it different than MAC_ADDR_STR_LEN, which results in EINVAL
> returned by ethaddr post process. Move the check for length earlier,
> to skip post processing, so nvmem could still present ethaddr value as
> a string if the value doesn't match MAC_ADDR_STR_LEN.
> 
> Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com>
> ---
> 
> v2
> Better clarification in commit message
> 
> v1
> Link: https://lore.kernel.org/all/20260319171236.48325-1-tmn505@terefe.re/
> 
>  drivers/nvmem/layouts/u-boot-env.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c
> index f27f387bb52a..dd3f541728bf 100644
> --- a/drivers/nvmem/layouts/u-boot-env.c
> +++ b/drivers/nvmem/layouts/u-boot-env.c
> @@ -38,9 +38,6 @@ static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, i
>  {
>  	u8 mac[ETH_ALEN];
>  
> -	if (bytes != MAC_ADDR_STR_LEN)
> -		return -EINVAL;
> -
>  	if (!mac_pton(buf, mac))
>  		return -EINVAL;
>  
> @@ -75,7 +72,7 @@ static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem
>  		info.offset = data_offset + value - data;
>  		info.bytes = strlen(value);
>  		info.np = of_get_child_by_name(dev->of_node, info.name);
> -		if (!strcmp(var, "ethaddr")) {
> +		if ((!strcmp(var, "ethaddr")) && (info.bytes == MAC_ADDR_STR_LEN)) {

if (!strcmp(var, "ethaddr") && info.bytes == MAC_ADDR_STR_LEN)

instead?


>  			info.raw_len = strlen(value);
>  			info.bytes = ETH_ALEN;
>  			info.read_post_process = u_boot_env_read_post_process_ethaddr;

Re: [PATCH v2] nvmem: layouts: u-boot-env: check earlier for ethaddr length

Posted by tomek@terefe.re 3 weeks, 4 days ago

W dniu 19.05.2026 o 17:55, Srinivas Kandagatla pisze:
> 
> 
> On 5/4/26 6:19 PM, Tomasz Maciej Nowak wrote:
>> From: Tomasz Maciej Nowak <tmn505@gmail.com>
>>
>> Unfortunately the ethaddr value in U-Boot environment might be enclosed
>> in single/double quotes or be something completely different. This can
>> make it different than MAC_ADDR_STR_LEN, which results in EINVAL
>> returned by ethaddr post process. Move the check for length earlier,
>> to skip post processing, so nvmem could still present ethaddr value as
>> a string if the value doesn't match MAC_ADDR_STR_LEN.
>>
>> Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com>
>> ---
>>
>> v2
>> Better clarification in commit message
>>
>> v1
>> Link: https://lore.kernel.org/all/20260319171236.48325-1-tmn505@terefe.re/
>>
>>  drivers/nvmem/layouts/u-boot-env.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c
>> index f27f387bb52a..dd3f541728bf 100644
>> --- a/drivers/nvmem/layouts/u-boot-env.c
>> +++ b/drivers/nvmem/layouts/u-boot-env.c
>> @@ -38,9 +38,6 @@ static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, i
>>  {
>>  	u8 mac[ETH_ALEN];
>>  
>> -	if (bytes != MAC_ADDR_STR_LEN)
>> -		return -EINVAL;
>> -
>>  	if (!mac_pton(buf, mac))
>>  		return -EINVAL;
>>  
>> @@ -75,7 +72,7 @@ static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem
>>  		info.offset = data_offset + value - data;
>>  		info.bytes = strlen(value);
>>  		info.np = of_get_child_by_name(dev->of_node, info.name);
>> -		if (!strcmp(var, "ethaddr")) {
>> +		if ((!strcmp(var, "ethaddr")) && (info.bytes == MAC_ADDR_STR_LEN)) {
> 
> if (!strcmp(var, "ethaddr") && info.bytes == MAC_ADDR_STR_LEN)
> 
> instead?

Ack, looks cleaner, will send v3 shortly.
> 
> 
>>  			info.raw_len = strlen(value);
>>  			info.bytes = ETH_ALEN;
>>  			info.read_post_process = u_boot_env_read_post_process_ethaddr;
> 

Regards

-- 
TMN