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://lore.kernel.org/r/[email protected] --- 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; } long smh_seek(long fd, long pos) --- base-commit: da47ddebd16a7e1047da8537fbf01558d2a89fcf change-id: 20251002-fs_semihosting-85d697fbfcad Best regards, -- Andrew Goodbody <[email protected]>
