On 5/18/23 17:17, Sean Anderson wrote:
On 5/17/23 06:23, Heinrich Schuchardt wrote:
The return value of smh_flen() is written to size and not to ret. But ret
is checked. We can avoid calling smh_flen() by setting maxsize to LONG_MAX
if it is not set yet.
Check input parameters.
Fixes: f676b45151c3 ("fs: Add semihosting filesystem")
Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
fs/semihostingfs.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/fs/semihostingfs.c b/fs/semihostingfs.c
index 96eb3349a2..8a7d4da884 100644
--- a/fs/semihostingfs.c
+++ b/fs/semihostingfs.c
@@ -25,6 +25,9 @@ static int smh_fs_read_at(const char *filename, loff_t pos,
void *buffer,
{
long fd, size, ret;
+ if (pos > LONG_MAX || maxsize > LONG_MAX)
Should be ULONG_MAX. The type should really be ulong but isn't.
+ return -EINVAL;
+
fd = smh_open(filename, MODE_READ | MODE_BINARY);
if (fd < 0)
return fd;
@@ -33,15 +36,8 @@ static int smh_fs_read_at(const char *filename, loff_t pos,
void *buffer,
smh_close(fd);
return ret;
}
- if (!maxsize) {
- size = smh_flen(fd);
- if (ret < 0) {
- smh_close(fd);
- return size;
- }
-
- maxsize = size;
- }
+ if (!maxsize)
+ maxsize = LONG_MAX;
Same here.
size = smh_read(fd, buffer, maxsize);
Thanks for reviewing.
Just changing to ULONG_MAX will not work, because smh_read() returns
errors as negative numbers.
We would need to change the interface of smh_read to:
int smh_read(long fd, void *memp, unsigned long *len)
Then we could return the error code and the number of bytes read separately.
https://developer.arm.com/documentation/dui0471/m/what-is-semihosting-/sys-read--0x06-
describes that SYS_READ may return less bytes than requested. This does
not signify that EOF has been reached but may simply reflect the
behavior of the underlying operating system (cf. 'man 2 read').
So shouldn't we loop here until all bytes are read?
Best regards
Heinrich
smh_close(fd);
This interface is nuts, but this patch does successfully implement it...
--Sean
