diff mbox series

[1/6] fwu: v2: perform some checks before reading metadata

Message ID 20240830114057.891069-2-sughosh.ganu@linaro.org
State New
Headers show
Series Miscellaneous FWU fixes | expand

Commit Message

Sughosh Ganu Aug. 30, 2024, 11:40 a.m. UTC
The version 2 of the FWU metadata has a top level structure, followed
by optional information on the updatable images. Perform some sanity
checks on some of the fields in the top level structure to determine
if the rest of the structure has to be read.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
 lib/fwu_updates/fwu_v2.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

Comments

Michal Simek Sept. 3, 2024, 1:51 p.m. UTC | #1
On 8/30/24 13:40, Sughosh Ganu wrote:
> The version 2 of the FWU metadata has a top level structure, followed
> by optional information on the updatable images. Perform some sanity
> checks on some of the fields in the top level structure to determine
> if the rest of the structure has to be read.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>   lib/fwu_updates/fwu_v2.c | 36 ++++++++++++++++++------------------
>   1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c
> index 108bc9bb4a..d0d8a25929 100644
> --- a/lib/fwu_updates/fwu_v2.c
> +++ b/lib/fwu_updates/fwu_v2.c
> @@ -58,24 +58,6 @@ static int fwu_mdata_sanity_checks(void)
>   	struct fwu_data *data = fwu_get_data();
>   	struct fwu_mdata *mdata = data->fwu_mdata;
>   
> -	if (mdata->version != FWU_MDATA_VERSION) {
> -		log_err("FWU metadata version %u. Expected value of %u\n",
> -			mdata->version, FWU_MDATA_VERSION);
> -		return -EINVAL;
> -	}
> -
> -	if (!mdata->desc_offset) {
> -		log_err("No image information provided with the Metadata. ");
> -		log_err("Image information expected in the metadata\n");
> -		return -EINVAL;
> -	}
> -
> -	if (mdata->desc_offset != 0x20) {
> -		log_err("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n",
> -			mdata->desc_offset);
> -		return -EINVAL;
> -	}
> -
>   	num_banks = fwu_get_fw_desc(mdata)->num_banks;
>   	num_images = fwu_get_fw_desc(mdata)->num_images;
>   
> @@ -238,6 +220,24 @@ int fwu_init(void)
>   		return ret;
>   	}
>   
> +	if (mdata.version != FWU_MDATA_VERSION) {
> +		log_err("FWU metadata version %u. Expected value of %u\n",
> +			mdata.version, FWU_MDATA_VERSION);
> +		return -EINVAL;
> +	}
> +
> +	if (!mdata.desc_offset) {
> +		log_err("No image information provided with the Metadata. ");
> +		log_err("Image information expected in the metadata\n");
> +		return -EINVAL;
> +	}
> +
> +	if (mdata.desc_offset != 0x20) {

In 2/6 you are introducing this macro
+#define FWU_IMG_DESC_OFFSET	0x20U

I think you can do it directly here and just use it in 2/6.


> +		log_err("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n",
> +			mdata.desc_offset);
> +		return -EINVAL;
> +	}
> +
>   	ret = fwu_mdata_copies_allocate(mdata.metadata_size);
>   	if (ret)
>   		return ret;

M
Sughosh Ganu Sept. 4, 2024, 7:18 a.m. UTC | #2
On Tue, 3 Sept 2024 at 19:22, Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 8/30/24 13:40, Sughosh Ganu wrote:
> > The version 2 of the FWU metadata has a top level structure, followed
> > by optional information on the updatable images. Perform some sanity
> > checks on some of the fields in the top level structure to determine
> > if the rest of the structure has to be read.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> >   lib/fwu_updates/fwu_v2.c | 36 ++++++++++++++++++------------------
> >   1 file changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c
> > index 108bc9bb4a..d0d8a25929 100644
> > --- a/lib/fwu_updates/fwu_v2.c
> > +++ b/lib/fwu_updates/fwu_v2.c
> > @@ -58,24 +58,6 @@ static int fwu_mdata_sanity_checks(void)
> >       struct fwu_data *data = fwu_get_data();
> >       struct fwu_mdata *mdata = data->fwu_mdata;
> >
> > -     if (mdata->version != FWU_MDATA_VERSION) {
> > -             log_err("FWU metadata version %u. Expected value of %u\n",
> > -                     mdata->version, FWU_MDATA_VERSION);
> > -             return -EINVAL;
> > -     }
> > -
> > -     if (!mdata->desc_offset) {
> > -             log_err("No image information provided with the Metadata. ");
> > -             log_err("Image information expected in the metadata\n");
> > -             return -EINVAL;
> > -     }
> > -
> > -     if (mdata->desc_offset != 0x20) {
> > -             log_err("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n",
> > -                     mdata->desc_offset);
> > -             return -EINVAL;
> > -     }
> > -
> >       num_banks = fwu_get_fw_desc(mdata)->num_banks;
> >       num_images = fwu_get_fw_desc(mdata)->num_images;
> >
> > @@ -238,6 +220,24 @@ int fwu_init(void)
> >               return ret;
> >       }
> >
> > +     if (mdata.version != FWU_MDATA_VERSION) {
> > +             log_err("FWU metadata version %u. Expected value of %u\n",
> > +                     mdata.version, FWU_MDATA_VERSION);
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (!mdata.desc_offset) {
> > +             log_err("No image information provided with the Metadata. ");
> > +             log_err("Image information expected in the metadata\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (mdata.desc_offset != 0x20) {
>
> In 2/6 you are introducing this macro
> +#define FWU_IMG_DESC_OFFSET    0x20U
>
> I think you can do it directly here and just use it in 2/6.

Will do. Thanks.

-sughosh

>
>
> > +             log_err("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n",
> > +                     mdata.desc_offset);
> > +             return -EINVAL;
> > +     }
> > +
> >       ret = fwu_mdata_copies_allocate(mdata.metadata_size);
> >       if (ret)
> >               return ret;
>
> M
Michal Simek Sept. 4, 2024, 8:43 a.m. UTC | #3
On 8/30/24 13:40, Sughosh Ganu wrote:
> The version 2 of the FWU metadata has a top level structure, followed
> by optional information on the updatable images. Perform some sanity
> checks on some of the fields in the top level structure to determine
> if the rest of the structure has to be read.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>   lib/fwu_updates/fwu_v2.c | 36 ++++++++++++++++++------------------
>   1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c
> index 108bc9bb4a..d0d8a25929 100644
> --- a/lib/fwu_updates/fwu_v2.c
> +++ b/lib/fwu_updates/fwu_v2.c
> @@ -58,24 +58,6 @@ static int fwu_mdata_sanity_checks(void)
>   	struct fwu_data *data = fwu_get_data();
>   	struct fwu_mdata *mdata = data->fwu_mdata;
>   
> -	if (mdata->version != FWU_MDATA_VERSION) {
> -		log_err("FWU metadata version %u. Expected value of %u\n",
> -			mdata->version, FWU_MDATA_VERSION);
> -		return -EINVAL;
> -	}
> -
> -	if (!mdata->desc_offset) {
> -		log_err("No image information provided with the Metadata. ");
> -		log_err("Image information expected in the metadata\n");
> -		return -EINVAL;
> -	}
> -
> -	if (mdata->desc_offset != 0x20) {
> -		log_err("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n",
> -			mdata->desc_offset);
> -		return -EINVAL;
> -	}
> -
>   	num_banks = fwu_get_fw_desc(mdata)->num_banks;
>   	num_images = fwu_get_fw_desc(mdata)->num_images;
>   
> @@ -238,6 +220,24 @@ int fwu_init(void)
>   		return ret;
>   	}
>   
> +	if (mdata.version != FWU_MDATA_VERSION) {
> +		log_err("FWU metadata version %u. Expected value of %u\n",
> +			mdata.version, FWU_MDATA_VERSION);
> +		return -EINVAL;
> +	}
> +
> +	if (!mdata.desc_offset) {
> +		log_err("No image information provided with the Metadata. ");
> +		log_err("Image information expected in the metadata\n");
> +		return -EINVAL;
> +	}

nit:
One more thing here. You are doing this just to report that desc_offset is not 0
and below it is equal to 0x20. I think you can check directly that it is 0x20.

M
diff mbox series

Patch

diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c
index 108bc9bb4a..d0d8a25929 100644
--- a/lib/fwu_updates/fwu_v2.c
+++ b/lib/fwu_updates/fwu_v2.c
@@ -58,24 +58,6 @@  static int fwu_mdata_sanity_checks(void)
 	struct fwu_data *data = fwu_get_data();
 	struct fwu_mdata *mdata = data->fwu_mdata;
 
-	if (mdata->version != FWU_MDATA_VERSION) {
-		log_err("FWU metadata version %u. Expected value of %u\n",
-			mdata->version, FWU_MDATA_VERSION);
-		return -EINVAL;
-	}
-
-	if (!mdata->desc_offset) {
-		log_err("No image information provided with the Metadata. ");
-		log_err("Image information expected in the metadata\n");
-		return -EINVAL;
-	}
-
-	if (mdata->desc_offset != 0x20) {
-		log_err("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n",
-			mdata->desc_offset);
-		return -EINVAL;
-	}
-
 	num_banks = fwu_get_fw_desc(mdata)->num_banks;
 	num_images = fwu_get_fw_desc(mdata)->num_images;
 
@@ -238,6 +220,24 @@  int fwu_init(void)
 		return ret;
 	}
 
+	if (mdata.version != FWU_MDATA_VERSION) {
+		log_err("FWU metadata version %u. Expected value of %u\n",
+			mdata.version, FWU_MDATA_VERSION);
+		return -EINVAL;
+	}
+
+	if (!mdata.desc_offset) {
+		log_err("No image information provided with the Metadata. ");
+		log_err("Image information expected in the metadata\n");
+		return -EINVAL;
+	}
+
+	if (mdata.desc_offset != 0x20) {
+		log_err("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n",
+			mdata.desc_offset);
+		return -EINVAL;
+	}
+
 	ret = fwu_mdata_copies_allocate(mdata.metadata_size);
 	if (ret)
 		return ret;