On 10/14/25 12:03, Andrew Goodbody wrote: > The sys call SYS_READ was being used in a way that did not 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. > > Signed-off-by: Andrew Goodbody <[email protected]> > --- > common/spl/spl_semihosting.c | 7 ++++--- > drivers/serial/serial_semihosting.c | 6 ++++-- > fs/semihostingfs.c | 9 +++++---- > include/semihosting.h | 8 ++++---- > lib/semihosting.c | 22 ++++++++++++++++++---- > 5 files changed, 35 insertions(+), 17 deletions(-) > > diff --git a/common/spl/spl_semihosting.c b/common/spl/spl_semihosting.c > index > f36863fe48d25859ee9af67a67140893b6e9e262..cff6f5351e265136b3effe483fd5d641fdd109f8 > 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, > 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..15f58d578b65f7dbbadce19093546f5bdb46c0c3 > 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) > @@ -43,10 +44,10 @@ static int smh_fs_read_at(const char *filename, loff_t > pos, void *buffer, > 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..42832d86c711ff04e91b6b739c80629253f6d30a > 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 > diff --git a/lib/semihosting.c b/lib/semihosting.c > index > 9be5bffd30124777c4f8110065e6cca5640312ac..e49b16b0aa34ab04b6c8949a3e7cab539fdbef82 > 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,23 @@ 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 if ((long)ret == -1) { > + *read_len = 0; > return smh_errno(); > - return len - ret; > + } else { > + /* Partial success */ > + *read_len = len - ret; > + return 0; > + } > +
There is no point. When are you going to read 3 GiB of file all at once on a 32-bit system? Any sane user will read multiple times into smaller buffers. So the correct fix is to add if (len > LONG_MAX) return -EOVERFLOW; to the beginning. --Sean
