[PATCH] spl: fix error handling of spl_fit_read

[Simon Glass]

+Sean Anderson too, for this

Hi,

On Fri, 16 Aug 2024 at 07:36, <mailinglist1 at johanneskirchmair.de> wrote:
>
> From: Johannes Kirchmair <johannes.kirchmair at skidata.com>
>
> Returning negative values from spl_fit_read leads to u-boot crashing.
> The return value of spl_fit_read is compared with an unsigned value.
> Returning negative values leads to the check not detecting the error.
> Not detecting the error leads to crashing.
>
> Returning zero in case of an reading error is fine. It indicates that
> nothing was red.

read

>
> Signed-off-by: Johannes Kirchmair <johannes.kirchmair at skidata.com>
> ---
>  common/spl/spl_fat.c | 2 +-
>  include/spl_load.h   | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>

Perhaps instead this should be fixed in _spl_load()? We don't want to
ignore errors, and returning -EIO is not as good as returning the
actual error received. The docs of struct spl_load_info indicate that
functions should return 0 on error, but that is somewhat surprising
behaviour, which is probably why people are not following it?

> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> index bd8aab253a..345bc55149 100644
> --- a/common/spl/spl_fat.c
> +++ b/common/spl/spl_fat.c
> @@ -53,7 +53,7 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
>
>         ret = fat_read_file(filename, buf, file_offset, size, &actread);
>         if (ret)
> -               return ret;
> +               return 0;
>
>         return actread;
>  }
> diff --git a/include/spl_load.h b/include/spl_load.h
> index 1c2b296c0a..b411d9daa1 100644
> --- a/include/spl_load.h
> +++ b/include/spl_load.h
> @@ -17,8 +17,8 @@ static inline int _spl_load(struct spl_image_info *spl_image,
>  {
>         struct legacy_img_hdr *header =
>                 spl_get_load_buffer(-sizeof(*header), sizeof(*header));
> -       ulong base_offset, image_offset, overhead;
> -       int read, ret;
> +       ulong base_offset, image_offset, overhead, read;
> +       int ret;
>
>         read = info->read(info, offset, ALIGN(sizeof(*header),
>                                               spl_get_bl_len(info)), header);
> --
> 2.34.1
>

Regards,
Simon