mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH master 1/3] common: machine_id: simplify early exit
@ 2022-08-08  6:20 Ahmad Fatoum
  2022-08-08  6:20 ` [PATCH master 2/3] common: machine_id: guard against digest algo being unavailable Ahmad Fatoum
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Ahmad Fatoum @ 2022-08-08  6:20 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

We don't need the goto if we haven't done anything to clean up anyway.
also globalvar_add_simple("machine_id", NULL) is a no-op when we have
just called globalvar_add_simple above with an actual argument.
It doesn't clean the parameter, nor should it, because the code is
executed for the successful code as well and there is nothing that can
fail that late.

No functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/machine_id.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/common/machine_id.c b/common/machine_id.c
index 6480806cd287..a530fdeb1da8 100644
--- a/common/machine_id.c
+++ b/common/machine_id.c
@@ -30,11 +30,11 @@ static int machine_id_set_globalvar(void)
 	unsigned char machine_id[SHA1_DIGEST_SIZE];
 	char hex_machine_id[MACHINE_ID_LENGTH];
 	char *env_machine_id;
-	int ret = 0;
+	int ret;
 
 	/* nothing to do if no hashable information provided */
 	if (!__machine_id_hashable)
-		goto out;
+		return 0;
 
 	digest = digest_alloc_by_algo(HASH_ALGO_SHA1);
 	ret = digest_init(digest);
@@ -58,8 +58,6 @@ static int machine_id_set_globalvar(void)
 	free(env_machine_id);
 
 out:
-	globalvar_add_simple("machine_id", NULL);
-
 	digest_free(digest);
 	return ret;
 
-- 
2.30.2




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH master 2/3] common: machine_id: guard against digest algo being unavailable
  2022-08-08  6:20 [PATCH master 1/3] common: machine_id: simplify early exit Ahmad Fatoum
@ 2022-08-08  6:20 ` Ahmad Fatoum
  2022-08-08 11:48   ` Sascha Hauer
  2022-08-08  6:20 ` [PATCH master 3/3] crypto: restrict digest algos implemented in ARM assembly to 32-bit Ahmad Fatoum
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Ahmad Fatoum @ 2022-08-08  6:20 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

All callsites of digest_alloc(_by_algo) check for NULL pointer before
proceeding except for machine_id_set_globalvar(). Fix this to fail
more gracefully instead of the crash I ran into.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/machine_id.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/machine_id.c b/common/machine_id.c
index a530fdeb1da8..189e74bd6796 100644
--- a/common/machine_id.c
+++ b/common/machine_id.c
@@ -30,14 +30,15 @@ static int machine_id_set_globalvar(void)
 	unsigned char machine_id[SHA1_DIGEST_SIZE];
 	char hex_machine_id[MACHINE_ID_LENGTH];
 	char *env_machine_id;
-	int ret;
+	int ret = -EOPNOTSUPP;
 
 	/* nothing to do if no hashable information provided */
 	if (!__machine_id_hashable)
 		return 0;
 
 	digest = digest_alloc_by_algo(HASH_ALGO_SHA1);
-	ret = digest_init(digest);
+	if (digest)
+		ret = digest_init(digest);
 	if (ret)
 		goto out;
 
-- 
2.30.2




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH master 3/3] crypto: restrict digest algos implemented in ARM assembly to 32-bit
  2022-08-08  6:20 [PATCH master 1/3] common: machine_id: simplify early exit Ahmad Fatoum
  2022-08-08  6:20 ` [PATCH master 2/3] common: machine_id: guard against digest algo being unavailable Ahmad Fatoum
@ 2022-08-08  6:20 ` Ahmad Fatoum
  2022-08-08 11:40 ` [PATCH master 1/3] common: machine_id: simplify early exit Sascha Hauer
  2022-08-09  4:41 ` Sascha Hauer
  3 siblings, 0 replies; 8+ messages in thread
From: Ahmad Fatoum @ 2022-08-08  6:20 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

These digest implementations are only built per arch/arm/Makefile when
!CONFIG_CPU_V8, so disallow selecting them when barebox is built
for 64-bit ARM.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 crypto/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index a2e03ae1096e..f32accb3d090 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -71,7 +71,7 @@ config DIGEST_HMAC_GENERIC
 
 config DIGEST_SHA1_ARM
 	tristate "SHA1 digest algorithm (ARM-asm)"
-	depends on ARM
+	depends on ARM && !CPU_V8
 	select HAVE_DIGEST_SHA1
 	help
 	  SHA-1 secure hash standard (FIPS 180-1/DFIPS 180-2) implemented
@@ -79,7 +79,7 @@ config DIGEST_SHA1_ARM
 
 config DIGEST_SHA256_ARM
 	tristate "SHA-224/256 digest algorithm (ARM-asm and NEON)"
-	depends on ARM
+	depends on ARM && !CPU_V8
 	select HAVE_DIGEST_SHA256
 	select HAVE_DIGEST_SHA224
 	help
-- 
2.30.2




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH master 1/3] common: machine_id: simplify early exit
  2022-08-08  6:20 [PATCH master 1/3] common: machine_id: simplify early exit Ahmad Fatoum
  2022-08-08  6:20 ` [PATCH master 2/3] common: machine_id: guard against digest algo being unavailable Ahmad Fatoum
  2022-08-08  6:20 ` [PATCH master 3/3] crypto: restrict digest algos implemented in ARM assembly to 32-bit Ahmad Fatoum
@ 2022-08-08 11:40 ` Sascha Hauer
  2022-08-08 11:43   ` Ahmad Fatoum
  2022-08-09  4:41 ` Sascha Hauer
  3 siblings, 1 reply; 8+ messages in thread
From: Sascha Hauer @ 2022-08-08 11:40 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Aug 08, 2022 at 08:20:08AM +0200, Ahmad Fatoum wrote:
> We don't need the goto if we haven't done anything to clean up anyway.
> also globalvar_add_simple("machine_id", NULL) is a no-op when we have
> just called globalvar_add_simple above with an actual argument.
> It doesn't clean the parameter, nor should it, because the code is
> executed for the successful code as well and there is nothing that can
> fail that late.
> 
> No functional change.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/machine_id.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/common/machine_id.c b/common/machine_id.c
> index 6480806cd287..a530fdeb1da8 100644
> --- a/common/machine_id.c
> +++ b/common/machine_id.c
> @@ -30,11 +30,11 @@ static int machine_id_set_globalvar(void)
>  	unsigned char machine_id[SHA1_DIGEST_SIZE];
>  	char hex_machine_id[MACHINE_ID_LENGTH];
>  	char *env_machine_id;
> -	int ret = 0;
> +	int ret;
>  
>  	/* nothing to do if no hashable information provided */
>  	if (!__machine_id_hashable)
> -		goto out;
> +		return 0;
>  
>  	digest = digest_alloc_by_algo(HASH_ALGO_SHA1);
>  	ret = digest_init(digest);
> @@ -58,8 +58,6 @@ static int machine_id_set_globalvar(void)
>  	free(env_machine_id);
>  
>  out:
> -	globalvar_add_simple("machine_id", NULL);

Without this patch we always created a global.machine_id variable,
either empty or with a value. With this patch we only create this
variable when it's non-empty. I think this change is ok as we also
don't have this variable when CONFIG_MACHINE_ID is disabled.

Still the "No functional change" doesn't hold true, so we should remove
it from the commit message.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH master 1/3] common: machine_id: simplify early exit
  2022-08-08 11:40 ` [PATCH master 1/3] common: machine_id: simplify early exit Sascha Hauer
@ 2022-08-08 11:43   ` Ahmad Fatoum
  0 siblings, 0 replies; 8+ messages in thread
From: Ahmad Fatoum @ 2022-08-08 11:43 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 08.08.22 13:40, Sascha Hauer wrote:
> On Mon, Aug 08, 2022 at 08:20:08AM +0200, Ahmad Fatoum wrote:
>> We don't need the goto if we haven't done anything to clean up anyway.
>> also globalvar_add_simple("machine_id", NULL) is a no-op when we have
>> just called globalvar_add_simple above with an actual argument.
>> It doesn't clean the parameter, nor should it, because the code is
>> executed for the successful code as well and there is nothing that can
>> fail that late.
>>
>> No functional change.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  common/machine_id.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/common/machine_id.c b/common/machine_id.c
>> index 6480806cd287..a530fdeb1da8 100644
>> --- a/common/machine_id.c
>> +++ b/common/machine_id.c
>> @@ -30,11 +30,11 @@ static int machine_id_set_globalvar(void)
>>  	unsigned char machine_id[SHA1_DIGEST_SIZE];
>>  	char hex_machine_id[MACHINE_ID_LENGTH];
>>  	char *env_machine_id;
>> -	int ret = 0;
>> +	int ret;
>>  
>>  	/* nothing to do if no hashable information provided */
>>  	if (!__machine_id_hashable)
>> -		goto out;
>> +		return 0;
>>  
>>  	digest = digest_alloc_by_algo(HASH_ALGO_SHA1);
>>  	ret = digest_init(digest);
>> @@ -58,8 +58,6 @@ static int machine_id_set_globalvar(void)
>>  	free(env_machine_id);
>>  
>>  out:
>> -	globalvar_add_simple("machine_id", NULL);
> 
> Without this patch we always created a global.machine_id variable,
> either empty or with a value. With this patch we only create this
> variable when it's non-empty. I think this change is ok as we also
> don't have this variable when CONFIG_MACHINE_ID is disabled.
> 
> Still the "No functional change" doesn't hold true, so we should remove
> it from the commit message.

You're right. I missed that. Can you replace the last line with:

  This slightly alters behavior: Whereas before $global.machine_id was
  always defined when CONFIG_MACHINE_ID is enabled, it's now only
  defined when it's non-empty.

Thanks,
Ahmad

> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH master 2/3] common: machine_id: guard against digest algo being unavailable
  2022-08-08  6:20 ` [PATCH master 2/3] common: machine_id: guard against digest algo being unavailable Ahmad Fatoum
@ 2022-08-08 11:48   ` Sascha Hauer
  2022-08-08 11:50     ` Ahmad Fatoum
  0 siblings, 1 reply; 8+ messages in thread
From: Sascha Hauer @ 2022-08-08 11:48 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Aug 08, 2022 at 08:20:09AM +0200, Ahmad Fatoum wrote:
> All callsites of digest_alloc(_by_algo) check for NULL pointer before
> proceeding except for machine_id_set_globalvar(). Fix this to fail
> more gracefully instead of the crash I ran into.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/machine_id.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/common/machine_id.c b/common/machine_id.c
> index a530fdeb1da8..189e74bd6796 100644
> --- a/common/machine_id.c
> +++ b/common/machine_id.c
> @@ -30,14 +30,15 @@ static int machine_id_set_globalvar(void)
>  	unsigned char machine_id[SHA1_DIGEST_SIZE];
>  	char hex_machine_id[MACHINE_ID_LENGTH];
>  	char *env_machine_id;
> -	int ret;
> +	int ret = -EOPNOTSUPP;
>  
>  	/* nothing to do if no hashable information provided */
>  	if (!__machine_id_hashable)
>  		return 0;
>  
>  	digest = digest_alloc_by_algo(HASH_ALGO_SHA1);

Had to read this twice to be sure it's correct. A plain

	if (!digest) {
		ret = -EOPNOTSUPP;
		goto out;
	}

seems easier to follow.

Sascha

> -	ret = digest_init(digest);
> +	if (digest)
> +		ret = digest_init(digest);
>  	if (ret)
>  		goto out;
>  
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH master 2/3] common: machine_id: guard against digest algo being unavailable
  2022-08-08 11:48   ` Sascha Hauer
@ 2022-08-08 11:50     ` Ahmad Fatoum
  0 siblings, 0 replies; 8+ messages in thread
From: Ahmad Fatoum @ 2022-08-08 11:50 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 08.08.22 13:48, Sascha Hauer wrote:
> On Mon, Aug 08, 2022 at 08:20:09AM +0200, Ahmad Fatoum wrote:
>> All callsites of digest_alloc(_by_algo) check for NULL pointer before
>> proceeding except for machine_id_set_globalvar(). Fix this to fail
>> more gracefully instead of the crash I ran into.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  common/machine_id.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/machine_id.c b/common/machine_id.c
>> index a530fdeb1da8..189e74bd6796 100644
>> --- a/common/machine_id.c
>> +++ b/common/machine_id.c
>> @@ -30,14 +30,15 @@ static int machine_id_set_globalvar(void)
>>  	unsigned char machine_id[SHA1_DIGEST_SIZE];
>>  	char hex_machine_id[MACHINE_ID_LENGTH];
>>  	char *env_machine_id;
>> -	int ret;
>> +	int ret = -EOPNOTSUPP;
>>  
>>  	/* nothing to do if no hashable information provided */
>>  	if (!__machine_id_hashable)
>>  		return 0;
>>  
>>  	digest = digest_alloc_by_algo(HASH_ALGO_SHA1);
> 
> Had to read this twice to be sure it's correct. A plain
> 
> 	if (!digest) {
> 		ret = -EOPNOTSUPP;
> 		goto out;
> 	}
> 
> seems easier to follow.

Fine by me. Can you fixup or should I?

Cheers,
Ahmad

> 
> Sascha
> 
>> -	ret = digest_init(digest);
>> +	if (digest)
>> +		ret = digest_init(digest);
>>  	if (ret)
>>  		goto out;
>>  
>> -- 
>> 2.30.2
>>
>>
>>
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH master 1/3] common: machine_id: simplify early exit
  2022-08-08  6:20 [PATCH master 1/3] common: machine_id: simplify early exit Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2022-08-08 11:40 ` [PATCH master 1/3] common: machine_id: simplify early exit Sascha Hauer
@ 2022-08-09  4:41 ` Sascha Hauer
  3 siblings, 0 replies; 8+ messages in thread
From: Sascha Hauer @ 2022-08-09  4:41 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Aug 08, 2022 at 08:20:08AM +0200, Ahmad Fatoum wrote:
> We don't need the goto if we haven't done anything to clean up anyway.
> also globalvar_add_simple("machine_id", NULL) is a no-op when we have
> just called globalvar_add_simple above with an actual argument.
> It doesn't clean the parameter, nor should it, because the code is
> executed for the successful code as well and there is nothing that can
> fail that late.
> 
> No functional change.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/machine_id.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/common/machine_id.c b/common/machine_id.c
> index 6480806cd287..a530fdeb1da8 100644
> --- a/common/machine_id.c
> +++ b/common/machine_id.c
> @@ -30,11 +30,11 @@ static int machine_id_set_globalvar(void)
>  	unsigned char machine_id[SHA1_DIGEST_SIZE];
>  	char hex_machine_id[MACHINE_ID_LENGTH];
>  	char *env_machine_id;
> -	int ret = 0;
> +	int ret;
>  
>  	/* nothing to do if no hashable information provided */
>  	if (!__machine_id_hashable)
> -		goto out;
> +		return 0;
>  
>  	digest = digest_alloc_by_algo(HASH_ALGO_SHA1);
>  	ret = digest_init(digest);
> @@ -58,8 +58,6 @@ static int machine_id_set_globalvar(void)
>  	free(env_machine_id);
>  
>  out:
> -	globalvar_add_simple("machine_id", NULL);
> -
>  	digest_free(digest);
>  	return ret;
>  
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-08-09  4:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-08  6:20 [PATCH master 1/3] common: machine_id: simplify early exit Ahmad Fatoum
2022-08-08  6:20 ` [PATCH master 2/3] common: machine_id: guard against digest algo being unavailable Ahmad Fatoum
2022-08-08 11:48   ` Sascha Hauer
2022-08-08 11:50     ` Ahmad Fatoum
2022-08-08  6:20 ` [PATCH master 3/3] crypto: restrict digest algos implemented in ARM assembly to 32-bit Ahmad Fatoum
2022-08-08 11:40 ` [PATCH master 1/3] common: machine_id: simplify early exit Sascha Hauer
2022-08-08 11:43   ` Ahmad Fatoum
2022-08-09  4:41 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox