[PATCH v4 2/2] fs: semihosting: Fixup smh_flen against overflow

[Sean Anderson]

On 10/14/25 12:59, Heinrich Schuchardt wrote:
> On 10/14/25 18: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 <andrew.goodbody at linaro.org>
>>> ---
>>>   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.
> 
> Dear Sean,
> 
> I prefer considerate discussion to NAKs.

I have discussed my reasoning on v2/v3. I want to make it clear that I
don't believe v4 is the correct path so it doesn't get merged
accidentally.

> Please, provide convincing reason for not supporting files beyond 2 GiB if you want this patch to be altered.

Outlined in v3.

--Sean