On 10/14/25 12:03, Andrew Goodbody wrote:
> The sys calls SYS_FLEN was being used in a way that did ot allow for
> file sizes > 2^31. Fixing this required changing the signature of the
> function to allow file sizes up to 2^32 - 2 and also being able to
> return errors that could be tested for.
> 
> This issue was found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <[email protected]>
> ---
>  common/spl/spl_semihosting.c |  6 +++---
>  fs/semihostingfs.c           | 14 ++++++++------
>  include/semihosting.h        |  5 +++--
>  lib/semihosting.c            |  6 ++++--
>  4 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/common/spl/spl_semihosting.c b/common/spl/spl_semihosting.c
> index 
> cff6f5351e265136b3effe483fd5d641fdd109f8..fb578a274d0d937b2ed5bd13e505bcefd4491799
>  100644
> --- a/common/spl/spl_semihosting.c
> +++ b/common/spl/spl_semihosting.c
> @@ -28,7 +28,8 @@ static int spl_smh_load_image(struct spl_image_info 
> *spl_image,
>  {
>       const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME;
>       int ret;
> -     long fd, len;
> +     long fd;
> +     size_t len;
>       struct spl_load_info load;
>  
>       fd = smh_open(filename, MODE_READ | MODE_BINARY);
> @@ -37,12 +38,11 @@ static int spl_smh_load_image(struct spl_image_info 
> *spl_image,
>               return fd;
>       }
>  
> -     ret = smh_flen(fd);
> +     ret = smh_flen(fd, &len);
>       if (ret < 0) {
>               log_debug("could not get length of image: %d\n", ret);
>               goto out;
>       }
> -     len = ret;
>  
>       spl_load_init(&load, smh_fit_read, &fd, 1);
>       ret = spl_load(spl_image, bootdev, &load, len, 0);
> diff --git a/fs/semihostingfs.c b/fs/semihostingfs.c
> index 
> 15f58d578b65f7dbbadce19093546f5bdb46c0c3..6a4d0aaaf2fdaf8da154e59677839a5c7a4889e9
>  100644
> --- a/fs/semihostingfs.c
> +++ b/fs/semihostingfs.c
> @@ -35,10 +35,10 @@ static int smh_fs_read_at(const char *filename, loff_t 
> pos, void *buffer,
>               return ret;
>       }
>       if (!maxsize) {
> -             size = smh_flen(fd);
> +             ret = smh_flen(fd, &size);
>               if (ret < 0) {
>                       smh_close(fd);
> -                     return size;
> +                     return ret;
>               }
>  
>               maxsize = size;
> @@ -79,17 +79,19 @@ static int smh_fs_write_at(const char *filename, loff_t 
> pos, void *buffer,
>  
>  int smh_fs_size(const char *filename, loff_t *result)
>  {
> -     long fd, size;
> +     long fd;
> +     size_t size;
> +     int ret;
>  
>       fd = smh_open(filename, MODE_READ | MODE_BINARY);
>       if (fd < 0)
>               return fd;
>  
> -     size = smh_flen(fd);
> +     ret = smh_flen(fd, &size);
>       smh_close(fd);
>  
> -     if (size < 0)
> -             return size;
> +     if (ret < 0)
> +             return ret;
>  
>       *result = size;
>       return 0;
> diff --git a/include/semihosting.h b/include/semihosting.h
> index 
> 42832d86c711ff04e91b6b739c80629253f6d30a..60535199b9532ce16620aea3ca1ea741c5b73766
>  100644
> --- a/include/semihosting.h
> +++ b/include/semihosting.h
> @@ -124,10 +124,11 @@ long smh_close(long fd);
>  /**
>   * smh_flen() - Get the length of a file
>   * @fd: A file descriptor returned from smh_open()
> + * @size: Pointer which will be updated with length of file on success
>   *
> - * Return: The length of the file, in bytes, or a negative error on failure
> + * Return: 0 on success or negative error on failure
>   */
> -long smh_flen(long fd);
> +int smh_flen(long fd, size_t *size);
>  
>  /**
>   * smh_seek() - Seek to a position in a file
> diff --git a/lib/semihosting.c b/lib/semihosting.c
> index 
> e49b16b0aa34ab04b6c8949a3e7cab539fdbef82..547a936e17f38204e840035a50f481e57d159592
>  100644
> --- a/lib/semihosting.c
> +++ b/lib/semihosting.c
> @@ -154,7 +154,7 @@ long smh_close(long fd)
>       return 0;
>  }
>  
> -long smh_flen(long fd)
> +int smh_flen(long fd, size_t *size)
>  {
>       long ret;
>  
> @@ -163,7 +163,9 @@ long smh_flen(long fd)
>       ret = smh_trap(SYSFLEN, &fd);
>       if (ret == -1)
>               return smh_errno();
> -     return ret;
> +
> +     *size = (size_t)ret;
> +     return 0;
>  }
>  
>  long smh_seek(long fd, long pos)
> 

Nack, for reasons outlined on v3.

This should just be 

    if (ret < 0)
        return ret == -1 ? smh_errno() : -EOVERFLOW;

--Sean

Reply via email to