On 10/2/25 10:32, Andrew Goodbody wrote: > The sys calls SYS_FLEN and SYS_READ were being used in a way that did > not allow for file sizes > 2^31. Fixing this required changing the > signatures of the functions smh_flen and smh_read 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]> > --- > Changes in v2: > - Fixup smh_flen and smh_read to allow for > 2^31 byte files > - Link to v1: > https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2fr%2f20251002%2dfs%5fsemihosting%2dv1%2d1%2d1124e6becc51%40linaro.org&umid=ee8de4df-2479-48c7-83ae-b302789e6a41&rct=1759415558&auth=d807158c60b7d2502abde8a2fc01f40662980862-944860761829276b5cf613a133926bf7544353ff > --- > common/spl/spl_semihosting.c | 13 +++++++------ > drivers/serial/serial_semihosting.c | 6 ++++-- > fs/semihostingfs.c | 13 +++++++------ > include/semihosting.h | 13 +++++++------ > lib/semihosting.c | 25 +++++++++++++++++++------ > 5 files changed, 44 insertions(+), 26 deletions(-) > > diff --git a/common/spl/spl_semihosting.c b/common/spl/spl_semihosting.c > index > f36863fe48d25859ee9af67a67140893b6e9e262..fb578a274d0d937b2ed5bd13e505bcefd4491799 > 100644 > --- a/common/spl/spl_semihosting.c > +++ b/common/spl/spl_semihosting.c > @@ -13,13 +13,14 @@ static ulong smh_fit_read(struct spl_load_info *load, > ulong file_offset, > ulong size, void *buf) > { > long fd = *(long *)load->priv; > - ulong ret; > + long ret; > + size_t bytes_read; > > if (smh_seek(fd, file_offset)) > return 0; > > - ret = smh_read(fd, buf, size); > - return ret < 0 ? 0 : ret; > + ret = smh_read(fd, buf, size, &bytes_read); > + return ret < 0 ? 0 : bytes_read; > } > > static int spl_smh_load_image(struct spl_image_info *spl_image, > @@ -27,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); > @@ -36,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/drivers/serial/serial_semihosting.c > b/drivers/serial/serial_semihosting.c > index > 56a5ec72428afdd97eccad739a46189380ba3e76..017b1f8eda84f5c25e09be7e970d0ebfdbf2ace0 > 100644 > --- a/drivers/serial/serial_semihosting.c > +++ b/drivers/serial/serial_semihosting.c > @@ -25,11 +25,12 @@ static int smh_serial_getc(struct udevice *dev) > { > char ch = 0; > struct smh_serial_priv *priv = dev_get_priv(dev); > + size_t bytes_read; > > if (priv->infd < 0) > return smh_getc(); > > - smh_read(priv->infd, &ch, sizeof(ch)); > + smh_read(priv->infd, &ch, sizeof(ch), &bytes_read); > return ch; > } > > @@ -140,11 +141,12 @@ static void smh_serial_setbrg(void) > static int smh_serial_getc(void) > { > char ch = 0; > + size_t bytes_read; > > if (infd < 0) > return smh_getc(); > > - smh_read(infd, &ch, sizeof(ch)); > + smh_read(infd, &ch, sizeof(ch), &bytes_read); > return ch; > } > > diff --git a/fs/semihostingfs.c b/fs/semihostingfs.c > index > 77e39ca407e4d240a1fd573497c5b6b908816454..71b6634b0660e32cc63f7fc60ec1af8d3248433a > 100644 > --- a/fs/semihostingfs.c > +++ b/fs/semihostingfs.c > @@ -23,7 +23,8 @@ int smh_fs_set_blk_dev(struct blk_desc *rbdd, struct > disk_partition *info) > static int smh_fs_read_at(const char *filename, loff_t pos, void *buffer, > loff_t maxsize, loff_t *actread) > { > - long fd, size, ret; > + long fd, ret; > + size_t size; > > fd = smh_open(filename, MODE_READ | MODE_BINARY); > if (fd < 0) > @@ -34,19 +35,19 @@ 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; > } > > - size = smh_read(fd, buffer, maxsize); > + ret = smh_read(fd, buffer, maxsize, &size); > smh_close(fd); > - if (size < 0) > - return size; > + if (ret < 0) > + return ret; > > *actread = size; > return 0; > diff --git a/include/semihosting.h b/include/semihosting.h > index > 4e844cbad87bb1ae6bb365f87f3e7a8aeea445f4..60535199b9532ce16620aea3ca1ea741c5b73766 > 100644 > --- a/include/semihosting.h > +++ b/include/semihosting.h > @@ -95,12 +95,12 @@ long smh_open(const char *fname, enum smh_open_mode mode); > * @fd: A file descriptor returned from smh_open() > * @memp: Pointer to a buffer of memory of at least @len bytes > * @len: The number of bytes to read > + * @read_len: Pointer which will be updated with actual bytes read on > success, > + * with 0 indicating %EOF > * > - * Return: > - * * The number of bytes read on success, with 0 indicating %EOF > - * * A negative error on failure > + * Return: 0 on success or negative error on failure > */ > -long smh_read(long fd, void *memp, size_t len); > +long smh_read(long fd, void *memp, size_t len, size_t *read_len); > > /** > * smh_write() - Write data to a file > @@ -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 > 9be5bffd30124777c4f8110065e6cca5640312ac..a0f42d41e4a7ad95023c684f6824bfc677ad25cd > 100644 > --- a/lib/semihosting.c > +++ b/lib/semihosting.c > @@ -93,9 +93,9 @@ struct smh_rdwr_s { > size_t len; > }; > > -long smh_read(long fd, void *memp, size_t len) > +long smh_read(long fd, void *memp, size_t len, size_t *read_len) > { > - long ret; > + size_t ret; > struct smh_rdwr_s read; > > debug("%s: fd %ld, memp %p, len %zu\n", __func__, fd, memp, len); > @@ -105,9 +105,20 @@ long smh_read(long fd, void *memp, size_t len) > read.len = len; > > ret = smh_trap(SYSREAD, &read); > - if (ret < 0) > + if (!ret) { > + /* Success */ > + *read_len = len; > + return 0; > + } else if (ret == len) { > + /* EOF */ > + *read_len = 0; > + return 0; > + } else { > + /* Partial success */ > + *read_len = len - ret; > return smh_errno(); > - return len - ret; > + } > + > } > > long smh_write(long fd, const void *memp, size_t len, ulong *written) > @@ -140,7 +151,7 @@ long smh_close(long fd) > return 0; > } > > -long smh_flen(long fd) > +int smh_flen(long fd, size_t *size) > { > long ret; > > @@ -149,7 +160,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;
This is completely wrong. long is 32-bits on 32-bit systems, so you cannot get the upper 64 bits just by casting to a size_t. --Sean
