8000 extmod/vfs_blockdev: Check block device function positive results. · micropython/micropython@4f6d4b2 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4f6d4b2

Browse files
projectgusdpgeorge
projectgus
authored and
dpgeorge
committed
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>
1 parent a2475ee commit 4f6d4b2

File tree

3 files changed

+228
-10
lines changed

3 files changed

+228
-10
lines changed

‎extmod/vfs_blockdev.c

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,19 @@
3232

3333
#if MICROPY_VFS
3434

35+
// Block device functions are expected to return 0 on success
36+
// and negative integer on errors. Check for positive integer
37+
// results as some callers (i.e. littlefs) will produce corrupt
38+
// results from these.
39+
static int mp_vfs_check_result(mp_obj_t ret) {
40+
if (ret == mp_const_none) {
41+
return 0;
42+
} else {
43+
int i = MP_OBJ_SMALL_INT_VALUE(ret);
44+
return i > 0 ? (-MP_EINVAL) : i;
45+
}
46+
}
47+
3548
void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
3649
mp_load_method(bdev, MP_QSTR_readblocks, self->readblocks);
3750
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
6679
self->readblocks[3] = MP_OBJ_FROM_PTR(&ar);
6780
self->readblocks[4] = MP_OBJ_NEW_SMALL_INT(block_off);
6881
mp_obj_t ret = mp_call_method_n_kw(3, 0, self->readblocks);
69-
if (ret == mp_const_none) {
70-
return 0;
71-
} else {
72-
return MP_OBJ_SMALL_INT_VALUE(ret);
73-
}
82+
return mp_vfs_check_result(ret);
7483
}
7584

7685
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
103112
self->writeblocks[3] = MP_OBJ_FROM_PTR(&ar);
104113
self->writeblocks[4] = MP_OBJ_NEW_SMALL_INT(block_off);
105114
mp_obj_t ret = mp_call_method_n_kw(3, 0, self->writeblocks);
106-
if (ret == mp_const_none) {
107-
return 0;
108-
} else {
109-
return MP_OBJ_SMALL_INT_VALUE(ret);
110-
}
115+
return mp_vfs_check_result(ret);
111116
}
112117

113118
mp_obj_t mp_vfs_blockdev_ioctl(mp_vfs_blockdev_t *self, uintptr_t cmd, uintptr_t arg) {

‎tests/extmod/vfs_blockdev_invalid.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# Tests where the block device returns invalid values
2+
3+
try:
4+
import vfs
5+
6+
vfs.VfsFat
7+
vfs.VfsLfs2
8+
except (ImportError, AttributeError):
9+
print("SKIP")
10+
raise SystemExit
11+
12+
13+
class RAMBlockDevice:
14+
ERASE_BLOCK_SIZE = 512
15+
16+
def __init__(self, blocks):
17+
self.data = bytearray(blocks * self.ERASE_BLOCK_SIZE)
18+
self.read_res = 0
19+
self.write_res = 0
20+
21+
def readblocks(self, block, buf, off=0):
22+
print("readblocks")
23+
addr = block * self.ERASE_BLOCK_SIZE + off
24+
for i in range(len(buf)):
25+
buf[i] = self.data[addr + i]
26+
return self.read_res
27+
28+
def writeblocks(self, block, buf, off=None):
29+
if off is None:
30+
# erase, then write
31+
off = 0
32+
addr = block * self.ERASE_BLOCK_SIZE + off
33+
for i in range(len(buf)):
34+
self.data[addr + i] = buf[i]
35+
return self.write_res
36+
37+
def ioctl(self, op, arg):
38+
if op == 4: # block count
39+
return len(self.data) // self.ERASE_BLOCK_SIZE
40+
if op == 5: # block size
41+
return self.ERASE_BLOCK_SIZE
42+
if op == 6: # erase block
43+
return 0
44+
45+
46+
try:
47+
bdev = RAMBlockDevice(50)
48+
except MemoryError:
49+
print("SKIP")
50+
raise SystemExit
51+
52+
53+
def test(vfs_class):
54+
print(vfs_class)
55+
56+
vfs_class.mkfs(bdev)
57+
fs = vfs_class(bdev)
58+
59+
with fs.open("test", "w") as f:
60+
f.write("a" * 64)
61+
62+
for res in (0, -5, 5, 33, "invalid"):
63+
# -5 is a legitimate negative failure (EIO), positive integer
64+
# is not
65+
66+
# This variant will fail on open
67+
bdev.read_res = res
68+
try:
69+
with fs.open("test", "r") as f:
70+
print("opened")
71+
except OSError as e:
72+
print("OSError", e)
73+
74+
# This variant should succeed on open, may fail on read
75+
# unless the filesystem cached the contents already
76+
bdev.read_res = 0
77+
try:
78+
with fs.open("test", "r") as f:
79+
bdev.read_res = res
80+
print("read 1", f.read(1))
81+
print("read rest", f.read())
82+
except OSError as e:
83+
print("OSError", e)
84+
85+
86+
test(vfs.VfsLfs2)
87+
test(vfs.VfsFat) # Looks like most failures of underlying device are ignored by VFAT currently

‎tests/extmod/vfs_blockdev_invalid.py.exp

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
<class 'VfsLfs2'>
2+
readblocks
3+
readblocks
4+
readblocks
5+
readblocks
6+
readblocks
7+
readblocks
8+
readblocks
9+
readblocks
10+
readblocks
11+
readblocks
12+
readblocks
13+
readblocks
14+
readblocks
15+
readblocks
16+
readblocks
17+
readblocks
18+
readblocks
19+
readblocks
20+
readblocks
21+
readblocks
22+
readblocks
23+
readblocks
24+
readblocks
25+
readblocks
26+
readblocks
27+
readblocks
28+
readblocks
29+
readblocks
30+
readblocks
31+
readblocks
32+
opened
33+
readblocks
34+
readblocks
35+
readblocks
36+
readblocks
37+
readblocks
38+
readblocks
39+
readblocks
40+
readblocks
41+
readblocks
42+
readblocks
43+
read 1 a
44+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
45+
readblocks
46+
OSError [Errno 5] EIO
47+
readblocks
48+
readblocks
49+
readblocks
50+
readblocks
51+
readblocks
52+
readblocks
53+
readblocks
54+
readblocks
55+
readblocks
56+
read 1 a
57+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
58+
readblocks
59+
OSError [Errno 22] EINVAL
60+
readblocks
61+
readblocks
62+
readblocks
63+
readblocks
64+
readblocks
65+
readblocks
66+
readblocks
67+
readblocks
68+
readblocks
69+
read 1 a
70+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
71+
readblocks
72+
OSError [Errno 22] EINVAL
73+
readblocks
74+
readblocks
75+
readblocks
76+
readblocks
77+
readblocks
78+
readblocks
79+
readblocks
80+
readblocks
81+
readblocks
82+
read 1 a
83+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
84+
readblocks
85+
OSError [Errno 22] EINVAL
86+
readblocks
87+
readblocks
88+
readblocks
89+
readblocks
90+
readblocks
91+
readblocks
92+
readblocks
93+
readblocks
94+
readblocks
95+
read 1 a
96+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
97+
<class 'VfsFat'>
98+
readblocks
99+
readblocks
100+
readblocks
101+
readblocks
102+
readblocks
103+
opened
104+
readblocks
105+
read 1 a
106+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
107+
readblocks
108+
opened
109+
readblocks
110+
read 1 a
111+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
112+
readblocks
113+
opened
114+
readblocks
115+
read 1 a
116+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
117+
readblocks
118+
opened
119+
readblocks
120+
read 1 a
121+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
122+
readblocks
123+
opened
124+
readblocks
125+
read 1 a
126+
read rest aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

0 commit comments

Comments
 (0)
0