Re: [U-Boot] [PATCH 03/25] remoteproc: elf_loader: Always check the validity of the image before loading
Hi Lokesh
I would prefer you keep the rproc_elf32_sanity_check() API available
(not static).
Some u-boot drivers (eg ti_power_proc.c) handle binary (raw) firmwares
while some other handle ELF format ones.
We may think about drivers that can handle both formats.
rproc_elf32_sanity_check() can then be used to check if the firmware
shall be ELF-decoded or simply memcpy'ed.
Anyway I agree with your proposal to call rproc_elf32_sanity_check()
from rproc_elf32_load_image()
BR
Fabien
On 28/08/2019 2:55 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.
>
> Now that rproc_elf32_sanity_check() is used only in rproc-elf-loader.c
> make it static.
>
> Signed-off-by: Lokesh Vutla
> ---
> drivers/remoteproc/rproc-elf-loader.c | 13 +
> drivers/remoteproc/stm32_copro.c | 9 +
> include/remoteproc.h | 20
> test/dm/remoteproc.c | 7 ++-
> 4 files changed, 16 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/remoteproc/rproc-elf-loader.c
> b/drivers/remoteproc/rproc-elf-loader.c
> index 7574ba3fb3..149aeafb85 100644
> --- a/drivers/remoteproc/rproc-elf-loader.c
> +++ b/drivers/remoteproc/rproc-elf-loader.c
> @@ -8,7 +8,7 @@
> #include
>
> /* Basic function to verify ELF32 image format */
> -int rproc_elf32_sanity_check(ulong addr, ulong size)
> +static int rproc_elf32_sanity_check(ulong addr, ulong size)
> {
> Elf32_Ehdr *ehdr;
> char class;
> @@ -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 ff82313bca..1760dc5aba 100644
> --- a/drivers/remoteproc/stm32_copro.c
> +++ b/drivers/remoteproc/stm32_copro.c
> @@ -149,14 +149,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 81f616a1f9..6657b39723 100644
> --- a/include/remoteproc.h
> +++ b/include/remoteproc.h
> @@ -202,25 +202,14 @@ int rproc_ping(int id);
>*/
> int rproc_is_running(int id);
>
> -/**
> - * rproc_elf32_sanity_check() - Verify if an image is a valid ELF32 one
> - *
> - * Check if a valid ELF32 image exists at the given memory location. Verify
> - * basic ELF32 format requirements like magic number and sections size.
> - *
> - * @addr:address of the image to verify
> - * @size:size of the image
> - * @return 0 if the image looks good, else appropriate error value.
> - */
> -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; }
> @@ -231,10 +220,9 @@ static inline int rproc_stop(int id) { return -ENOSYS; }
> static inline int rproc_reset(int id) { return -ENOSYS; }
> static inline int rproc_ping(int id) { return -ENOSYS; }
> 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,
> -
[U-Boot] [PATCH 03/25] remoteproc: elf_loader: Always check the validity of the image before loading
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.
Now that rproc_elf32_sanity_check() is used only in rproc-elf-loader.c
make it static.
Signed-off-by: Lokesh Vutla
---
drivers/remoteproc/rproc-elf-loader.c | 13 +
drivers/remoteproc/stm32_copro.c | 9 +
include/remoteproc.h | 20
test/dm/remoteproc.c | 7 ++-
4 files changed, 16 insertions(+), 33 deletions(-)
diff --git a/drivers/remoteproc/rproc-elf-loader.c
b/drivers/remoteproc/rproc-elf-loader.c
index 7574ba3fb3..149aeafb85 100644
--- a/drivers/remoteproc/rproc-elf-loader.c
+++ b/drivers/remoteproc/rproc-elf-loader.c
@@ -8,7 +8,7 @@
#include
/* Basic function to verify ELF32 image format */
-int rproc_elf32_sanity_check(ulong addr, ulong size)
+static int rproc_elf32_sanity_check(ulong addr, ulong size)
{
Elf32_Ehdr *ehdr;
char class;
@@ -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 ff82313bca..1760dc5aba 100644
--- a/drivers/remoteproc/stm32_copro.c
+++ b/drivers/remoteproc/stm32_copro.c
@@ -149,14 +149,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 81f616a1f9..6657b39723 100644
--- a/include/remoteproc.h
+++ b/include/remoteproc.h
@@ -202,25 +202,14 @@ int rproc_ping(int id);
*/
int rproc_is_running(int id);
-/**
- * rproc_elf32_sanity_check() - Verify if an image is a valid ELF32 one
- *
- * Check if a valid ELF32 image exists at the given memory location. Verify
- * basic ELF32 format requirements like magic number and sections size.
- *
- * @addr: address of the image to verify
- * @size: size of the image
- * @return 0 if the image looks good, else appropriate error value.
- */
-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; }
@@ -231,10 +220,9 @@ static inline int rproc_stop(int id) { return -ENOSYS; }
static inline int rproc_reset(int id) { return -ENOSYS; }
static inline int rproc_ping(int id) { return -ENOSYS; }
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..fb3b615fc3 100644
--- a/test/dm/remoteproc.c
+++ b/test/dm/remoteproc.c
@@ -171,18 +171,15 @@ 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, (un
