[PATCH v3] fs: semihosting: Fixup smh_flen and smh_read against overflow
Andrew Goodbody
andrew.goodbody at linaro.org
Fri Oct 3 11:03:57 CEST 2025
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 <andrew.goodbody at linaro.org>
---
Changes in v3:
- V2 missed fixing up one use of smh_flen, fix it
- Link to v2: https://lore.kernel.org/r/20251002-fs_semihosting-v2-1-c1783e4188d8@linaro.org
Changes in v2:
- Fixup smh_flen and smh_read to allow for > 2^31 byte files
- Link to v1: https://lore.kernel.org/r/20251002-fs_semihosting-v1-1-1124e6becc51@linaro.org
---
common/spl/spl_semihosting.c | 13 +++++++------
drivers/serial/serial_semihosting.c | 6 ++++--
fs/semihostingfs.c | 23 +++++++++++++----------
include/semihosting.h | 13 +++++++------
lib/semihosting.c | 25 +++++++++++++++++++------
5 files changed, 50 insertions(+), 30 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..6a4d0aaaf2fdaf8da154e59677839a5c7a4889e9 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;
@@ -78,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 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 <andrew.goodbody at linaro.org>
More information about the U-Boot
mailing list