mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] fixup! pblimage: pblimage: Add LS1028a support
@ 2024-01-10  9:15 Ahmad Fatoum
  2024-01-10  9:17 ` Ahmad Fatoum
  2024-01-10 14:34 ` Sascha Hauer
  0 siblings, 2 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2024-01-10  9:15 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

soc_index is not only used for comparisons, but also as index into the
socs[] array. However, the array starts with SOC_TYPE_LS1046A and the
enum with SOC_TYPE_INVALID leading to socs[SOC_TYPE_LS1046A].soc
== SOC_TYPE_LS1028A and socs[SOC_TYPE_LS1028A] to overflow the array.

This broke LS1046A boot and LS1028A seems to have only worked by chance,
because the memory after socs[] happened to be zeroed.

Fix this by reordering the enum and while at it remove the redundancy in
having both the index and the soc member be the same value.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 scripts/pblimage.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/scripts/pblimage.c b/scripts/pblimage.c
index 8cb473d5bcf2..ef09b0f96084 100644
--- a/scripts/pblimage.c
+++ b/scripts/pblimage.c
@@ -57,30 +57,28 @@ static uint32_t pbi_crc_cmd1;
 static uint32_t pbi_crc_cmd2;
 
 enum soc_type {
-	SOC_TYPE_INVALID,
+	SOC_TYPE_INVALID = -1,
 	SOC_TYPE_LS1046A,
 	SOC_TYPE_LS1028A,
 };
 
 struct soc_type_entry {
 	const char *name;
-	enum soc_type soc;
 	bool big_endian;
 };
 
 static struct soc_type_entry socs[] = {
-	{
+	[SOC_TYPE_LS1046A] = {
 		.name = "ls1046a",
-		.soc = SOC_TYPE_LS1046A,
 		.big_endian = true,
-	}, {
+	},
+	[SOC_TYPE_LS1028A] = {
 		.name = "ls1028a",
-		.soc = SOC_TYPE_LS1028A,
 		.big_endian = false,
 	},
 };
 
-static enum soc_type soc_type;
+static enum soc_type soc_type = SOC_TYPE_INVALID;
 
 static char *rcwfile;
 static char *pbifile;
@@ -413,7 +411,7 @@ int main(int argc, char *argv[])
 
 	for (i = 0; i < ARRAY_SIZE(socs); i++) {
 		if (!strcmp(socs[i].name, cputypestr)) {
-			soc_type = socs[i].soc;
+			soc_type = i;
 			break;
 		}
 	}
-- 
2.39.2




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

* Re: [PATCH] fixup! pblimage: pblimage: Add LS1028a support
  2024-01-10  9:15 [PATCH] fixup! pblimage: pblimage: Add LS1028a support Ahmad Fatoum
@ 2024-01-10  9:17 ` Ahmad Fatoum
  2024-01-10 14:34 ` Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2024-01-10  9:17 UTC (permalink / raw)
  To: barebox

Title is wrong (only a single pblimage: ), but shouldn't matter anyway,
as commit should be squashed.

On 10.01.24 10:15, Ahmad Fatoum wrote:
> soc_index is not only used for comparisons, but also as index into the
> socs[] array. However, the array starts with SOC_TYPE_LS1046A and the
> enum with SOC_TYPE_INVALID leading to socs[SOC_TYPE_LS1046A].soc
> == SOC_TYPE_LS1028A and socs[SOC_TYPE_LS1028A] to overflow the array.
> 
> This broke LS1046A boot and LS1028A seems to have only worked by chance,
> because the memory after socs[] happened to be zeroed.
> 
> Fix this by reordering the enum and while at it remove the redundancy in
> having both the index and the soc member be the same value.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  scripts/pblimage.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/scripts/pblimage.c b/scripts/pblimage.c
> index 8cb473d5bcf2..ef09b0f96084 100644
> --- a/scripts/pblimage.c
> +++ b/scripts/pblimage.c
> @@ -57,30 +57,28 @@ static uint32_t pbi_crc_cmd1;
>  static uint32_t pbi_crc_cmd2;
>  
>  enum soc_type {
> -	SOC_TYPE_INVALID,
> +	SOC_TYPE_INVALID = -1,
>  	SOC_TYPE_LS1046A,
>  	SOC_TYPE_LS1028A,
>  };
>  
>  struct soc_type_entry {
>  	const char *name;
> -	enum soc_type soc;
>  	bool big_endian;
>  };
>  
>  static struct soc_type_entry socs[] = {
> -	{
> +	[SOC_TYPE_LS1046A] = {
>  		.name = "ls1046a",
> -		.soc = SOC_TYPE_LS1046A,
>  		.big_endian = true,
> -	}, {
> +	},
> +	[SOC_TYPE_LS1028A] = {
>  		.name = "ls1028a",
> -		.soc = SOC_TYPE_LS1028A,
>  		.big_endian = false,
>  	},
>  };
>  
> -static enum soc_type soc_type;
> +static enum soc_type soc_type = SOC_TYPE_INVALID;
>  
>  static char *rcwfile;
>  static char *pbifile;
> @@ -413,7 +411,7 @@ int main(int argc, char *argv[])
>  
>  	for (i = 0; i < ARRAY_SIZE(socs); i++) {
>  		if (!strcmp(socs[i].name, cputypestr)) {
> -			soc_type = socs[i].soc;
> +			soc_type = i;
>  			break;
>  		}
>  	}

-- 
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] 3+ messages in thread

* Re: [PATCH] fixup! pblimage: pblimage: Add LS1028a support
  2024-01-10  9:15 [PATCH] fixup! pblimage: pblimage: Add LS1028a support Ahmad Fatoum
  2024-01-10  9:17 ` Ahmad Fatoum
@ 2024-01-10 14:34 ` Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Sascha Hauer @ 2024-01-10 14:34 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Wed, Jan 10, 2024 at 10:15:38AM +0100, Ahmad Fatoum wrote:
> soc_index is not only used for comparisons, but also as index into the
> socs[] array. However, the array starts with SOC_TYPE_LS1046A and the
> enum with SOC_TYPE_INVALID leading to socs[SOC_TYPE_LS1046A].soc
> == SOC_TYPE_LS1028A and socs[SOC_TYPE_LS1028A] to overflow the array.
> 
> This broke LS1046A boot and LS1028A seems to have only worked by chance,
> because the memory after socs[] happened to be zeroed.
> 
> Fix this by reordering the enum and while at it remove the redundancy in
> having both the index and the soc member be the same value.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  scripts/pblimage.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/scripts/pblimage.c b/scripts/pblimage.c
> index 8cb473d5bcf2..ef09b0f96084 100644
> --- a/scripts/pblimage.c
> +++ b/scripts/pblimage.c
> @@ -57,30 +57,28 @@ static uint32_t pbi_crc_cmd1;
>  static uint32_t pbi_crc_cmd2;
>  
>  enum soc_type {
> -	SOC_TYPE_INVALID,
> +	SOC_TYPE_INVALID = -1,
>  	SOC_TYPE_LS1046A,
>  	SOC_TYPE_LS1028A,
>  };
>  
>  struct soc_type_entry {
>  	const char *name;
> -	enum soc_type soc;
>  	bool big_endian;
>  };
>  
>  static struct soc_type_entry socs[] = {
> -	{
> +	[SOC_TYPE_LS1046A] = {
>  		.name = "ls1046a",
> -		.soc = SOC_TYPE_LS1046A,
>  		.big_endian = true,
> -	}, {
> +	},
> +	[SOC_TYPE_LS1028A] = {
>  		.name = "ls1028a",
> -		.soc = SOC_TYPE_LS1028A,
>  		.big_endian = false,
>  	},
>  };
>  
> -static enum soc_type soc_type;
> +static enum soc_type soc_type = SOC_TYPE_INVALID;
>  
>  static char *rcwfile;
>  static char *pbifile;
> @@ -413,7 +411,7 @@ int main(int argc, char *argv[])
>  
>  	for (i = 0; i < ARRAY_SIZE(socs); i++) {
>  		if (!strcmp(socs[i].name, cputypestr)) {
> -			soc_type = socs[i].soc;
> +			soc_type = i;
>  			break;
>  		}
>  	}
> -- 
> 2.39.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] 3+ messages in thread

end of thread, other threads:[~2024-01-10 14:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-10  9:15 [PATCH] fixup! pblimage: pblimage: Add LS1028a support Ahmad Fatoum
2024-01-10  9:17 ` Ahmad Fatoum
2024-01-10 14:34 ` Sascha Hauer

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