Skip to content

extmod: Check vfs block device invalid return values, increase error handling #13572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 25 additions & 33 deletions extmod/vfs_blockdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,31 +46,38 @@ void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
}
}

// Helper function to minimise code size of read/write functions
// note the n_args argument is moved to the end for further code size reduction (args keep same position in caller and callee).
static int mp_vfs_blockdev_call_rw(mp_obj_t *args, size_t block_num, size_t block_off, size_t len, void *buf, size_t n_args) {
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, buf};
args[2] = MP_OBJ_NEW_SMALL_INT(block_num);
args[3] = MP_OBJ_FROM_PTR(&ar);
args[4] = MP_OBJ_NEW_SMALL_INT(block_off); // ignored for n_args == 2
mp_obj_t ret = mp_call_method_n_kw(n_args, 0, args);

if (ret == mp_const_none) {
return 0;
} else {
// 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.
int i = MP_OBJ_SMALL_INT_VALUE(ret);
return i > 0 ? (-MP_EINVAL) : i;
}
}

int mp_vfs_blockdev_read(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, uint8_t *buf) {
if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
mp_uint_t (*f)(uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->readblocks[2];
return f(buf, block_num, num_blocks);
} else {
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, num_blocks *self->block_size, buf};
self->readblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
self->readblocks[3] = MP_OBJ_FROM_PTR(&ar);
mp_call_method_n_kw(2, 0, self->readblocks);
// TODO handle error return
return 0;
return mp_vfs_blockdev_call_rw(self->readblocks, block_num, 0, num_blocks * self->block_size, buf, 2);
}
}

int mp_vfs_blockdev_read_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t block_off, size_t len, uint8_t *buf) {
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, buf};
self->readblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
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_blockdev_call_rw(self->readblocks, block_num, block_off, len, buf, 3);
}

int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, const uint8_t *buf) {
Expand All @@ -83,12 +90,7 @@ int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_
mp_uint_t (*f)(const uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->writeblocks[2];
return f(buf, block_num, num_blocks);
} else {
mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, num_blocks *self->block_size, (void *)buf};
self->writeblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
self->writeblocks[3] = MP_OBJ_FROM_PTR(&ar);
mp_call_method_n_kw(2, 0, self->writeblocks);
// TODO handle error return
return 0;
return mp_vfs_blockdev_call_rw(self->writeblocks, block_num, 0, num_blocks * self->block_size, (void *)buf, 2);
}
}

Expand All @@ -97,17 +99,7 @@ int mp_vfs_blockdev_write_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t
// read-only block device
return -MP_EROFS;
}

mp_obj_array_t ar = {{&mp_type_bytearray}, BYTEARRAY_TYPECODE, 0, len, (void *)buf};
self->writeblocks[2] = MP_OBJ_NEW_SMALL_INT(block_num);
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_blockdev_call_rw(self->writeblocks, block_num, block_off, len, (void *)buf, 3);
}

mp_obj_t mp_vfs_blockdev_ioctl(mp_vfs_blockdev_t *self, uintptr_t cmd, uintptr_t arg) {
Expand Down
89 changes: 89 additions & 0 deletions tests/extmod/vfs_blockdev_invalid.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# 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)
bdev.read_res = 0 # reset function results
bdev.write_res = 0

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)
126 changes: 126 additions & 0 deletions tests/extmod/vfs_blockdev_invalid.py.exp
Original file line number Diff line number Diff line change
@@ -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
OSError [Errno 5] EIO
readblocks
readblocks
OSError [Errno 5] EIO
readblocks
OSError [Errno 5] EIO
readblocks
readblocks
OSError [Errno 5] EIO
readblocks
OSError [Errno 5] EIO
readblocks
readblocks
OSError [Errno 5] EIO
readblocks
OSError [Errno 5] EIO
readblocks
readblocks
OSError [Errno 5] EIO
Loading