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 <angus@redyak.com.au>

extmod/vfs_blockdev.c

@@ -32,6 +32,19 @@
 
 #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) {
+    if (ret == mp_const_none) {
+        return 0;
+    } else {
+        int i = MP_OBJ_SMALL_INT_VALUE(ret);
+        return i > 0 ? (-MP_EINVAL) : 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);
@@ -66,11 +79,7 @@ int mp_vfs_blockdev_read_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t b
     self->readblocks[3] = MP_OBJ_FROM_PTR(&ar);
     self->readblocks[4] = MP_OBJ_NEW_SMALL_INT(block_off);
     mp_obj_t ret = mp_call_method_n_kw(3, 0, self->readblocks);
-    if (ret == mp_const_none) {
-        return 0;
-    } else {
-        return MP_OBJ_SMALL_INT_VALUE(ret);
-    }
+    return mp_vfs_check_result(ret);
 }
 
 int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, const uint8_t *buf) {
@@ -103,11 +112,7 @@ int mp_vfs_blockdev_write_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t
     self->writeblocks[3] = MP_OBJ_FROM_PTR(&ar);
     self->writeblocks[4] = MP_OBJ_NEW_SMALL_INT(block_off);
     mp_obj_t ret = mp_call_method_n_kw(3, 0, self->writeblocks);
-    if (ret == mp_const_none) {
-        return 0;
-    } else {
-        return MP_OBJ_SMALL_INT_VALUE(ret);
-    }
+    return mp_vfs_check_result(ret);
 }
 
 mp_obj_t mp_vfs_blockdev_ioctl(mp_vfs_blockdev_t *self, uintptr_t cmd, uintptr_t arg) {

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

tests/extmod/vfs_blockdev_invalid.py.exp

@@ -0,0 +1,126 @@
+<class 'VfsLfs2'>
+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
+<class 'VfsFat'>
+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