[PATCH v3] barebox: Fix excessive loading of FIT images

[PATCH v3] barebox: Fix excessive loading of FIT images

[Ahmad Fatoum]

From: Christian Melki <christian.melki@t2data.com>

Barebox doesn't use the FIT image size from the header
when loading FIT images. It bluntly assumes that the FIT image
is equal to the file size. Which would be true if the
FIT image is a file. But if it's situated on a raw device,
then barebox proceeds to load the entire contents of that
raw device, only to conclude that it only needed parts of it.
Fix it.

Cc: Daniel Brát <danek.brat@gmail.com>
Signed-off-by: Christian Melki <christian.melki@t2data.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v3 was here: https://lore.barebox.org/barebox/20220729205441.9512-1-danek.brat@gmail.com/
v2 -> v3:
 - restrict change to bootm_open_fit
 - use cached data in struct image_data
v1 -> v2:
 - use fdt32_to_cpu to read the totalsize from header
---
 common/bootm.c      | 9 ++++++++-
 common/image-fit.c  | 7 ++++---
 include/image-fit.h | 2 +-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c
index fb1ed36a26dc..91a6e1688674 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -522,13 +522,20 @@ static int bootm_open_os_uimage(struct image_data *data)
 static int bootm_open_fit(struct image_data *data)
 {
 	struct fit_handle *fit;
+	struct fdt_header *header;
 	static const char *kernel_img = "kernel";
+	size_t flen, hlen;
 	int ret;
 
 	if (!IS_ENABLED(CONFIG_FITIMAGE))
 		return 0;
 
-	fit = fit_open(data->os_file, data->verbose, data->verify);
+	header = (struct fdt_header *)data->os_header;
+	flen = bootm_get_os_size(data);
+	hlen = fdt32_to_cpu(header->totalsize);
+
+	fit = fit_open(data->os_file, data->verbose, data->verify,
+		       min(flen, hlen));
 	if (IS_ERR(fit)) {
 		pr_err("Loading FIT image %s failed with: %pe\n", data->os_file, fit);
 		return PTR_ERR(fit);
diff --git a/common/image-fit.c b/common/image-fit.c
index 3e6e7fbd6d12..9bea62bb34a0 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -827,6 +827,7 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
  * @filename:	The filename of the FIT image
  * @verbose:	If true, be more verbose
  * @verify:	The verify mode
+ * @max_size:	maximum length to read from file
  *
  * This opens a FIT image found in @filename. The returned handle is used as
  * context for the other FIT functions.
@@ -834,7 +835,7 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
  * Return: A handle to a FIT image or a ERR_PTR
  */
 struct fit_handle *fit_open(const char *filename, bool verbose,
-			    enum bootm_verify verify)
+			    enum bootm_verify verify, loff_t max_size)
 {
 	struct fit_handle *handle;
 	int ret;
@@ -845,8 +846,8 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 	handle->verify = verify;
 
 	ret = read_file_2(filename, &handle->size, &handle->fit_alloc,
-			  FILESIZE_MAX);
-	if (ret) {
+			  max_size);
+	if (ret && ret != -EFBIG) {
 		pr_err("unable to read %s: %s\n", filename, strerror(-ret));
 		return ERR_PTR(ret);
 	}
diff --git a/include/image-fit.h b/include/image-fit.h
index f21545988e16..0b8e94bf4635 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -23,7 +23,7 @@ struct fit_handle {
 };
 
 struct fit_handle *fit_open(const char *filename, bool verbose,
-			    enum bootm_verify verify);
+			    enum bootm_verify verify, loff_t max_size);
 struct fit_handle *fit_open_buf(const void *buf, size_t len, bool verbose,
 				enum bootm_verify verify);
 void *fit_open_configuration(struct fit_handle *handle, const char *name);
-- 
2.39.2

Re: [PATCH v3] barebox: Fix excessive loading of FIT images

[Sascha Hauer]

On Fri, May 19, 2023 at 02:10:28PM +0200, Ahmad Fatoum wrote:
> From: Christian Melki <christian.melki@t2data.com>
> 
> Barebox doesn't use the FIT image size from the header
> when loading FIT images. It bluntly assumes that the FIT image
> is equal to the file size. Which would be true if the
> FIT image is a file. But if it's situated on a raw device,
> then barebox proceeds to load the entire contents of that
> raw device, only to conclude that it only needed parts of it.
> Fix it.
> 
> Cc: Daniel Brát <danek.brat@gmail.com>
> Signed-off-by: Christian Melki <christian.melki@t2data.com>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---

Applied, thanks

Sascha

> v3 was here: https://lore.barebox.org/barebox/20220729205441.9512-1-danek.brat@gmail.com/
> v2 -> v3:
>  - restrict change to bootm_open_fit
>  - use cached data in struct image_data
> v1 -> v2:
>  - use fdt32_to_cpu to read the totalsize from header
> ---
>  common/bootm.c      | 9 ++++++++-
>  common/image-fit.c  | 7 ++++---
>  include/image-fit.h | 2 +-
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/common/bootm.c b/common/bootm.c
> index fb1ed36a26dc..91a6e1688674 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -522,13 +522,20 @@ static int bootm_open_os_uimage(struct image_data *data)
>  static int bootm_open_fit(struct image_data *data)
>  {
>  	struct fit_handle *fit;
> +	struct fdt_header *header;
>  	static const char *kernel_img = "kernel";
> +	size_t flen, hlen;
>  	int ret;
>  
>  	if (!IS_ENABLED(CONFIG_FITIMAGE))
>  		return 0;
>  
> -	fit = fit_open(data->os_file, data->verbose, data->verify);
> +	header = (struct fdt_header *)data->os_header;
> +	flen = bootm_get_os_size(data);
> +	hlen = fdt32_to_cpu(header->totalsize);
> +
> +	fit = fit_open(data->os_file, data->verbose, data->verify,
> +		       min(flen, hlen));
>  	if (IS_ERR(fit)) {
>  		pr_err("Loading FIT image %s failed with: %pe\n", data->os_file, fit);
>  		return PTR_ERR(fit);
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 3e6e7fbd6d12..9bea62bb34a0 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -827,6 +827,7 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
>   * @filename:	The filename of the FIT image
>   * @verbose:	If true, be more verbose
>   * @verify:	The verify mode
> + * @max_size:	maximum length to read from file
>   *
>   * This opens a FIT image found in @filename. The returned handle is used as
>   * context for the other FIT functions.
> @@ -834,7 +835,7 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
>   * Return: A handle to a FIT image or a ERR_PTR
>   */
>  struct fit_handle *fit_open(const char *filename, bool verbose,
> -			    enum bootm_verify verify)
> +			    enum bootm_verify verify, loff_t max_size)
>  {
>  	struct fit_handle *handle;
>  	int ret;
> @@ -845,8 +846,8 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
>  	handle->verify = verify;
>  
>  	ret = read_file_2(filename, &handle->size, &handle->fit_alloc,
> -			  FILESIZE_MAX);
> -	if (ret) {
> +			  max_size);
> +	if (ret && ret != -EFBIG) {
>  		pr_err("unable to read %s: %s\n", filename, strerror(-ret));
>  		return ERR_PTR(ret);
>  	}
> diff --git a/include/image-fit.h b/include/image-fit.h
> index f21545988e16..0b8e94bf4635 100644
> --- a/include/image-fit.h
> +++ b/include/image-fit.h
> @@ -23,7 +23,7 @@ struct fit_handle {
>  };
>  
>  struct fit_handle *fit_open(const char *filename, bool verbose,
> -			    enum bootm_verify verify);
> +			    enum bootm_verify verify, loff_t max_size);
>  struct fit_handle *fit_open_buf(const void *buf, size_t len, bool verbose,
>  				enum bootm_verify verify);
>  void *fit_open_configuration(struct fit_handle *handle, const char *name);
> -- 
> 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 |