From d02cd5c0ad0a48e03d50d8119f629b8453e779b5 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 31 Jan 2024 18:18:14 +1100 Subject: [PATCH] extmod/vfs_blockdev: Check block device function positive results. A positive result here can result in eventual memory corruption as littlefs expects the result of a cache read/write function to be 0 or a negative integer for an error. Closes #13046 This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- extmod/vfs_blockdev.c | 16 ++- tests/extmod/vfs_blockdev_invalid.py | 87 ++++++++++++++++ tests/extmod/vfs_blockdev_invalid.py.exp | 126 +++++++++++++++++++++++ 3 files changed, 227 insertions(+), 2 deletions(-) create mode 100644 tests/extmod/vfs_blockdev_invalid.py create mode 100644 tests/extmod/vfs_blockdev_invalid.py.exp diff --git a/extmod/vfs_blockdev.c b/extmod/vfs_blockdev.c index 57c83b4289..e98ed5abeb 100644 --- a/extmod/vfs_blockdev.c +++ b/extmod/vfs_blockdev.c @@ -32,6 +32,18 @@ #if MICROPY_VFS +// Block device functions are expected to return 0 on success +// and negative integer on errors. Check for positive integer +// results as some callers (i.e. littlefs) will produce corrupt +// results from these. +static int mp_vfs_check_result(mp_obj_t ret) { + int i = MP_OBJ_SMALL_INT_VALUE(ret); + if (i > 0) { + mp_raise_OSError(MP_EINVAL); + } + return i; +} + void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) { mp_load_method(bdev, MP_QSTR_readblocks, self->readblocks); mp_load_method_maybe(bdev, MP_QSTR_writeblocks, self->writeblocks); @@ -69,7 +81,7 @@ int mp_vfs_blockdev_read_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t b if (ret == mp_const_none) { return 0; } else { - return MP_OBJ_SMALL_INT_VALUE(ret); + return mp_vfs_check_result(ret); } } @@ -106,7 +118,7 @@ int mp_vfs_blockdev_write_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t if (ret == mp_const_none) { return 0; } else { - return MP_OBJ_SMALL_INT_VALUE(ret); + return mp_vfs_check_result(ret); } } diff --git a/tests/extmod/vfs_blockdev_invalid.py b/tests/extmod/vfs_blockdev_invalid.py new file mode 100644 index 0000000000..80afc6414d --- /dev/null +++ b/tests/extmod/vfs_blockdev_invalid.py @@ -0,0 +1,87 @@ +# Tests where the block device returns invalid values + +try: + import vfs + + vfs.VfsFat + vfs.VfsLfs2 +except (ImportError, AttributeError): + print("SKIP") + raise SystemExit + + +class RAMBlockDevice: + ERASE_BLOCK_SIZE = 512 + + def __init__(self, blocks): + self.data = bytearray(blocks * self.ERASE_BLOCK_SIZE) + self.read_res = 0 + self.write_res = 0 + + def readblocks(self, block, buf, off=0): + print("readblocks") + addr = block * self.ERASE_BLOCK_SIZE + off + for i in range(len(buf)): + buf[i] = self.data[addr + i] + return self.read_res + + def writeblocks(self, block, buf, off=None): + if off is None: + # erase, then write + off = 0 + addr = block * self.ERASE_BLOCK_SIZE + off + for i in range(len(buf)): + self.data[addr + i] = buf[i] + return self.write_res + + def ioctl(self, op, arg): + if op == 4: # block count + return len(self.data) // self.ERASE_BLOCK_SIZE + if op == 5: # block size + return self.ERASE_BLOCK_SIZE + if op == 6: # erase block + return 0 + + +try: + bdev = RAMBlockDevice(50) +except MemoryError: + print("SKIP") + raise SystemExit + + +def test(vfs_class): + print(vfs_class) + + vfs_class.mkfs(bdev) + fs = vfs_class(bdev) + + with fs.open("test", "w") as f: + f.write("a" * 64) + + for res in (0, -5, 5, 33, "invalid"): + # -5 is a legitimate negative failure (EIO), positive integer + # is not + + # This variant will fail on open + bdev.read_res = res + try: + with fs.open("test", "r") as f: + print("opened") + except OSError as e: + print("OSError", e) + + # This variant should succeed on open, may fail on read + # unless the filesystem cached the contents already + bdev.read_res = 0 + try: + with fs.open("test", "r") as f: + bdev.read_res = res + print("read 1", f.read(1)) + print("read rest", f.read()) + except OSError as e: + print("OSError", e) + + +test(vfs.VfsLfs2) +test(vfs.VfsFat) # Looks like most failures of underlying device are ignored by VFAT currently diff --git a/tests/extmod/vfs_blockdev_invalid.py.exp b/tests/extmod/vfs_blockdev_invalid.py.exp new file mode 100644 index 0000000000..992111719e --- /dev/null +++ b/tests/extmod/vfs_blockdev_invalid.py.exp @@ -0,0 +1,126 @@ + +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +opened +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +readblocks +OSError [Errno 5] EIO +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +readblocks +OSError [Errno 22] EINVAL +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +readblocks +OSError [Errno 22] EINVAL +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +readblocks +OSError [Errno 22] EINVAL +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + +readblocks +readblocks +readblocks +readblocks +readblocks +opened +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +readblocks +opened +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +readblocks +opened +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +readblocks +opened +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +readblocks +opened +readblocks +read 1 a +read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa