On 14/10/2025 17:21, Sean Anderson wrote:
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

I am not going to spend any more time on this. There is a real bug that is addressed by V1.
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
Please consider applying that.

Andrew

Reply via email to