diff mbox series

[U-Boot,v2,03/26] remoteproc: elf_loader: Always check the validity of the image before loading

Message ID 20190904103151.20121-4-lokeshvutla@ti.com
State Accepted
Commit 14d963d1b51fc7a57b8d561588f163d7cae8ed81
Delegated to: Tom Rini
Headers show
Series remoteproc: Add support for R5F and DSP processors | expand

Commit Message

Lokesh Vutla Sept. 4, 2019, 10:31 a.m. UTC
rproc_elf32_load_image() rely on user to send a valid address for elf loading.
Instead do a sanity check on the address passed by user. This will help
all rproc elf users to not call sanity_check explicitly before calling
elf_loading.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/remoteproc/rproc-elf-loader.c | 11 ++++++++---
 drivers/remoteproc/stm32_copro.c      |  9 +--------
 include/remoteproc.h                  |  6 ++++--
 test/dm/remoteproc.c                  |  5 +----
 4 files changed, 14 insertions(+), 17 deletions(-)

Comments

Fabien DESSENNE Sept. 4, 2019, 3:05 p.m. UTC | #1
Hi Lokesh


On 04/09/2019 12:31 PM, Lokesh Vutla wrote:
> rproc_elf32_load_image() rely on user to send a valid address for elf loading.
> Instead do a sanity check on the address passed by user. This will help
> all rproc elf users to not call sanity_check explicitly before calling
> elf_loading.
>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Reviewed-by: Fabien Dessenne <fabien.dessenne@st.com>
> ---
>   drivers/remoteproc/rproc-elf-loader.c | 11 ++++++++---
>   drivers/remoteproc/stm32_copro.c      |  9 +--------
>   include/remoteproc.h                  |  6 ++++--
>   test/dm/remoteproc.c                  |  5 +----
>   4 files changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
> index 7574ba3fb3..238f51d3b5 100644
> --- a/drivers/remoteproc/rproc-elf-loader.c
> +++ b/drivers/remoteproc/rproc-elf-loader.c
> @@ -64,13 +64,18 @@ int rproc_elf32_sanity_check(ulong addr, ulong size)
>   	return 0;
>   }
>   
> -/* A very simple elf loader, assumes the image is valid */
> -int rproc_elf32_load_image(struct udevice *dev, unsigned long addr)
> +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
>   {
>   	Elf32_Ehdr *ehdr; /* Elf header structure pointer */
>   	Elf32_Phdr *phdr; /* Program header structure pointer */
>   	const struct dm_rproc_ops *ops;
> -	unsigned int i;
> +	unsigned int i, ret;
> +
> +	ret =  rproc_elf32_sanity_check(addr, size);
> +	if (ret) {
> +		dev_err(dev, "Invalid ELF32 Image %d\n", ret);
> +		return ret;
> +	}
>   
>   	ehdr = (Elf32_Ehdr *)addr;
>   	phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
> diff --git a/drivers/remoteproc/stm32_copro.c b/drivers/remoteproc/stm32_copro.c
> index 71895daf9c..40bba37211 100644
> --- a/drivers/remoteproc/stm32_copro.c
> +++ b/drivers/remoteproc/stm32_copro.c
> @@ -155,14 +155,7 @@ static int stm32_copro_load(struct udevice *dev, ulong addr, ulong size)
>   		return ret;
>   	}
>   
> -	/* Support only ELF32 image */
> -	ret = rproc_elf32_sanity_check(addr, size);
> -	if (ret) {
> -		dev_err(dev, "Invalid ELF32 image (%d)\n", ret);
> -		return ret;
> -	}
> -
> -	return rproc_elf32_load_image(dev, addr);
> +	return rproc_elf32_load_image(dev, addr, size);
>   }
>   
>   /**
> diff --git a/include/remoteproc.h b/include/remoteproc.h
> index dbff1ce3cf..1ef88be7f7 100644
> --- a/include/remoteproc.h
> +++ b/include/remoteproc.h
> @@ -218,9 +218,10 @@ int rproc_elf32_sanity_check(ulong addr, ulong size);
>    * rproc_elf32_load_image() - load an ELF32 image
>    * @dev:	device loading the ELF32 image
>    * @addr:	valid ELF32 image address
> + * @size:	size of the image
>    * @return 0 if the image is successfully loaded, else appropriate error value.
>    */
> -int rproc_elf32_load_image(struct udevice *dev, unsigned long addr);
> +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size);
>   #else
>   static inline int rproc_init(void) { return -ENOSYS; }
>   static inline int rproc_dev_init(int id) { return -ENOSYS; }
> @@ -234,7 +235,8 @@ static inline int rproc_is_running(int id) { return -ENOSYS; }
>   static inline int rproc_elf32_sanity_check(ulong addr,
>   					   ulong size) { return -ENOSYS; }
>   static inline int rproc_elf32_load_image(struct udevice *dev,
> -					 unsigned long addr) { return -ENOSYS; }
> +					 unsigned long addr, ulong size)
> +{ return -ENOSYS; }
>   #endif
>   
>   #endif	/* _RPROC_H_ */
> diff --git a/test/dm/remoteproc.c b/test/dm/remoteproc.c
> index a2c4be7c27..c77361c8f4 100644
> --- a/test/dm/remoteproc.c
> +++ b/test/dm/remoteproc.c
> @@ -171,11 +171,8 @@ static int dm_test_remoteproc_elf(struct unit_test_state *uts)
>   	ut_assertnonnull(loaded_firmware);
>   	memset(loaded_firmware, 0, loaded_firmware_size);
>   
> -	/* Verify valid ELF format */
> -	ut_assertok(rproc_elf32_sanity_check((ulong)valid_elf32, size));
> -
>   	/* Load firmware in loaded_firmware, and verify it */
> -	ut_assertok(rproc_elf32_load_image(dev, (unsigned long)valid_elf32));
> +	ut_assertok(rproc_elf32_load_image(dev, (ulong)valid_elf32, size));
>   	ut_assertok(memcmp(loaded_firmware, valid_elf32, loaded_firmware_size));
>   	unmap_physmem(loaded_firmware, MAP_NOCACHE);
>
Tom Rini Oct. 12, 2019, 8:24 p.m. UTC | #2
On Wed, Sep 04, 2019 at 04:01:28PM +0530, Lokesh Vutla wrote:

> rproc_elf32_load_image() rely on user to send a valid address for elf loading.
> Instead do a sanity check on the address passed by user. This will help
> all rproc elf users to not call sanity_check explicitly before calling
> elf_loading.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Reviewed-by: Fabien Dessenne <fabien.dessenne@st.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
index 7574ba3fb3..238f51d3b5 100644
--- a/drivers/remoteproc/rproc-elf-loader.c
+++ b/drivers/remoteproc/rproc-elf-loader.c
@@ -64,13 +64,18 @@  int rproc_elf32_sanity_check(ulong addr, ulong size)
 	return 0;
 }
 
-/* A very simple elf loader, assumes the image is valid */
-int rproc_elf32_load_image(struct udevice *dev, unsigned long addr)
+int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size)
 {
 	Elf32_Ehdr *ehdr; /* Elf header structure pointer */
 	Elf32_Phdr *phdr; /* Program header structure pointer */
 	const struct dm_rproc_ops *ops;
-	unsigned int i;
+	unsigned int i, ret;
+
+	ret =  rproc_elf32_sanity_check(addr, size);
+	if (ret) {
+		dev_err(dev, "Invalid ELF32 Image %d\n", ret);
+		return ret;
+	}
 
 	ehdr = (Elf32_Ehdr *)addr;
 	phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
diff --git a/drivers/remoteproc/stm32_copro.c b/drivers/remoteproc/stm32_copro.c
index 71895daf9c..40bba37211 100644
--- a/drivers/remoteproc/stm32_copro.c
+++ b/drivers/remoteproc/stm32_copro.c
@@ -155,14 +155,7 @@  static int stm32_copro_load(struct udevice *dev, ulong addr, ulong size)
 		return ret;
 	}
 
-	/* Support only ELF32 image */
-	ret = rproc_elf32_sanity_check(addr, size);
-	if (ret) {
-		dev_err(dev, "Invalid ELF32 image (%d)\n", ret);
-		return ret;
-	}
-
-	return rproc_elf32_load_image(dev, addr);
+	return rproc_elf32_load_image(dev, addr, size);
 }
 
 /**
diff --git a/include/remoteproc.h b/include/remoteproc.h
index dbff1ce3cf..1ef88be7f7 100644
--- a/include/remoteproc.h
+++ b/include/remoteproc.h
@@ -218,9 +218,10 @@  int rproc_elf32_sanity_check(ulong addr, ulong size);
  * rproc_elf32_load_image() - load an ELF32 image
  * @dev:	device loading the ELF32 image
  * @addr:	valid ELF32 image address
+ * @size:	size of the image
  * @return 0 if the image is successfully loaded, else appropriate error value.
  */
-int rproc_elf32_load_image(struct udevice *dev, unsigned long addr);
+int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size);
 #else
 static inline int rproc_init(void) { return -ENOSYS; }
 static inline int rproc_dev_init(int id) { return -ENOSYS; }
@@ -234,7 +235,8 @@  static inline int rproc_is_running(int id) { return -ENOSYS; }
 static inline int rproc_elf32_sanity_check(ulong addr,
 					   ulong size) { return -ENOSYS; }
 static inline int rproc_elf32_load_image(struct udevice *dev,
-					 unsigned long addr) { return -ENOSYS; }
+					 unsigned long addr, ulong size)
+{ return -ENOSYS; }
 #endif
 
 #endif	/* _RPROC_H_ */
diff --git a/test/dm/remoteproc.c b/test/dm/remoteproc.c
index a2c4be7c27..c77361c8f4 100644
--- a/test/dm/remoteproc.c
+++ b/test/dm/remoteproc.c
@@ -171,11 +171,8 @@  static int dm_test_remoteproc_elf(struct unit_test_state *uts)
 	ut_assertnonnull(loaded_firmware);
 	memset(loaded_firmware, 0, loaded_firmware_size);
 
-	/* Verify valid ELF format */
-	ut_assertok(rproc_elf32_sanity_check((ulong)valid_elf32, size));
-
 	/* Load firmware in loaded_firmware, and verify it */
-	ut_assertok(rproc_elf32_load_image(dev, (unsigned long)valid_elf32));
+	ut_assertok(rproc_elf32_load_image(dev, (ulong)valid_elf32, size));
 	ut_assertok(memcmp(loaded_firmware, valid_elf32, loaded_firmware_size));
 	unmap_physmem(loaded_firmware, MAP_NOCACHE);