From fa746e299086b40fe4c6e7943b62a281ccec8efe Mon Sep 17 00:00:00 2001 From: matejcik Date: Wed, 26 Feb 2020 14:18:41 +0100 Subject: [PATCH 1/5] core/fatfs: rework low-level FatFS API Instead of having possibly multiple FatFS objects, each with its own `fs` struct, there is one global static fs_instance. This is to match the mode of operation of ff.c, which assumes a global list of mounts, and all functions operate on the global based on path. Methods of FatFS were converted to functions on the fatfs module. fatfs.unmount() does not call ff.c's unmount, but simply invalidates fs_instance. This is basically what ff.c would do, except without messing with ff.c's global list of mounts. --- .../extmod/modtrezorio/modtrezorio-fatfs.h | 185 +++++++++--------- .../extmod/modtrezorio/modtrezorio-sdcard.h | 2 + core/embed/extmod/modtrezorio/modtrezorio.c | 2 +- core/mocks/generated/trezorio/__init__.pyi | 125 ------------ core/mocks/generated/trezorio/fatfs.pyi | 142 ++++++++++++++ 5 files changed, 237 insertions(+), 219 deletions(-) create mode 100644 core/mocks/generated/trezorio/fatfs.pyi diff --git a/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h b/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h index fbd3fb1e8..a2bfcebc0 100644 --- a/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h +++ b/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h @@ -27,7 +27,12 @@ #include "sdcard.h" // clang-format on -/// package: trezorio.__init__ +/// package: trezorio.fatfs + +static FATFS fs_instance; + +bool _fatfs_instance_is_mounted() { return fs_instance.fs_type != 0; } +void _fatfs_unmount_instance() { fs_instance.fs_type = 0; } DSTATUS disk_initialize(BYTE pdrv) { return disk_status(pdrv); } @@ -315,33 +320,13 @@ STATIC const mp_obj_type_t mod_trezorio_FatFSDir_type = { .iternext = mod_trezorio_FatFSDir_iternext, }; -/// class FatFS: -/// """ -/// Class encapsulating FAT filesystem -/// """ -typedef struct _mp_obj_FatFS_t { - mp_obj_base_t base; - FATFS fs; -} mp_obj_FatFS_t; - -/// def __init__(self) -> None: -/// """ -/// """ -STATIC mp_obj_t mod_trezorio_FatFS_make_new(const mp_obj_type_t *type, - size_t n_args, size_t n_kw, - const mp_obj_t *args) { - mp_arg_check_num(n_args, n_kw, 0, 0, false); - mp_obj_FatFS_t *o = m_new_obj(mp_obj_FatFS_t); - o->base.type = type; - return MP_OBJ_FROM_PTR(o); -} +/// mock:global -/// def open(self, path: str, flags: str) -> FatFSFile: +/// def open(path: str, flags: str) -> FatFSFile: /// """ /// Open or create a file /// """ -STATIC mp_obj_t mod_trezorio_FatFS_open(mp_obj_t self, mp_obj_t path, - mp_obj_t flags) { +STATIC mp_obj_t mod_trezorio_fatfs_open(mp_obj_t path, mp_obj_t flags) { mp_buffer_info_t _path, _flags; mp_get_buffer_raise(path, &_path, MP_BUFFER_READ); mp_get_buffer_raise(flags, &_flags, MP_BUFFER_READ); @@ -376,14 +361,14 @@ STATIC mp_obj_t mod_trezorio_FatFS_open(mp_obj_t self, mp_obj_t path, f->fp = fp; return f; } -STATIC MP_DEFINE_CONST_FUN_OBJ_3(mod_trezorio_FatFS_open_obj, - mod_trezorio_FatFS_open); +STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_fatfs_open_obj, + mod_trezorio_fatfs_open); -/// def listdir(self, path: str) -> FatFSDir: +/// def listdir(path: str) -> FatFSDir: /// """ /// List a directory (return generator) /// """ -STATIC mp_obj_t mod_trezorio_FatFS_listdir(mp_obj_t self, mp_obj_t path) { +STATIC mp_obj_t mod_trezorio_fatfs_listdir(mp_obj_t path) { mp_buffer_info_t _path; mp_get_buffer_raise(path, &_path, MP_BUFFER_READ); DIR dp; @@ -396,19 +381,19 @@ STATIC mp_obj_t mod_trezorio_FatFS_listdir(mp_obj_t self, mp_obj_t path) { d->dp = dp; return d; } -STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_FatFS_listdir_obj, - mod_trezorio_FatFS_listdir); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_fatfs_listdir_obj, + mod_trezorio_fatfs_listdir); -/// def mkdir(self, path: str, exist_ok: bool=False) -> None: +/// def mkdir(path: str, exist_ok: bool=False) -> None: /// """ /// Create a sub directory /// """ -STATIC mp_obj_t mod_trezorio_FatFS_mkdir(size_t n_args, const mp_obj_t *args) { +STATIC mp_obj_t mod_trezorio_fatfs_mkdir(size_t n_args, const mp_obj_t *args) { mp_buffer_info_t path; - mp_get_buffer_raise(args[1], &path, MP_BUFFER_READ); + mp_get_buffer_raise(args[0], &path, MP_BUFFER_READ); FRESULT res = f_mkdir(path.buf); // directory exists and exist_ok is True, return without failure - if (res == FR_EXIST && n_args > 2 && args[2] == mp_const_true) { + if (res == FR_EXIST && n_args > 1 && args[1] == mp_const_true) { return mp_const_none; } if (res != FR_OK) { @@ -416,14 +401,14 @@ STATIC mp_obj_t mod_trezorio_FatFS_mkdir(size_t n_args, const mp_obj_t *args) { } return mp_const_none; } -STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorio_FatFS_mkdir_obj, 2, 3, - mod_trezorio_FatFS_mkdir); +STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorio_fatfs_mkdir_obj, 1, 2, + mod_trezorio_fatfs_mkdir); -/// def unlink(self, path: str) -> None: +/// def unlink(path: str) -> None: /// """ /// Delete an existing file or directory /// """ -STATIC mp_obj_t mod_trezorio_FatFS_unlink(mp_obj_t self, mp_obj_t path) { +STATIC mp_obj_t mod_trezorio_fatfs_unlink(mp_obj_t path) { mp_buffer_info_t _path; mp_get_buffer_raise(path, &_path, MP_BUFFER_READ); FRESULT res = f_unlink(_path.buf); @@ -432,14 +417,14 @@ STATIC mp_obj_t mod_trezorio_FatFS_unlink(mp_obj_t self, mp_obj_t path) { } return mp_const_none; } -STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_FatFS_unlink_obj, - mod_trezorio_FatFS_unlink); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_fatfs_unlink_obj, + mod_trezorio_fatfs_unlink); -/// def stat(self, path: str) -> Tuple[int, str, str]: +/// def stat(path: str) -> Tuple[int, str, str]: /// """ /// Get file status /// """ -STATIC mp_obj_t mod_trezorio_FatFS_stat(mp_obj_t self, mp_obj_t path) { +STATIC mp_obj_t mod_trezorio_fatfs_stat(mp_obj_t path) { mp_buffer_info_t _path; mp_get_buffer_raise(path, &_path, MP_BUFFER_READ); FILINFO info; @@ -449,15 +434,14 @@ STATIC mp_obj_t mod_trezorio_FatFS_stat(mp_obj_t self, mp_obj_t path) { } return filinfo_to_tuple(&info); } -STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_FatFS_stat_obj, - mod_trezorio_FatFS_stat); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_fatfs_stat_obj, + mod_trezorio_fatfs_stat); /// def rename(self, oldpath: str, newpath: str) -> None: /// """ /// Rename/Move a file or directory /// """ -STATIC mp_obj_t mod_trezorio_FatFS_rename(mp_obj_t self, mp_obj_t oldpath, - mp_obj_t newpath) { +STATIC mp_obj_t mod_trezorio_fatfs_rename(mp_obj_t oldpath, mp_obj_t newpath) { mp_buffer_info_t _oldpath, _newpath; mp_get_buffer_raise(oldpath, &_oldpath, MP_BUFFER_READ); mp_get_buffer_raise(newpath, &_newpath, MP_BUFFER_READ); @@ -467,44 +451,52 @@ STATIC mp_obj_t mod_trezorio_FatFS_rename(mp_obj_t self, mp_obj_t oldpath, } return mp_const_none; } -STATIC MP_DEFINE_CONST_FUN_OBJ_3(mod_trezorio_FatFS_rename_obj, - mod_trezorio_FatFS_rename); +STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_fatfs_rename_obj, + mod_trezorio_fatfs_rename); -/// def mount(self) -> None: +/// def mount() -> None: /// """ -/// Mount/Unmount a logical drive +/// Mount the SD card filesystem. /// """ -STATIC mp_obj_t mod_trezorio_FatFS_mount(mp_obj_t self) { - mp_obj_FatFS_t *o = MP_OBJ_TO_PTR(self); - FRESULT res = f_mount(&(o->fs), "", 1); +STATIC mp_obj_t mod_trezorio_fatfs_mount() { + FRESULT res = f_mount(&fs_instance, "", 1); if (res != FR_OK) { mp_raise_OSError(fresult_to_errno_table[res]); } return mp_const_none; } -STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_FatFS_mount_obj, - mod_trezorio_FatFS_mount); +STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorio_fatfs_mount_obj, + mod_trezorio_fatfs_mount); -/// def unmount(self) -> None: +/// def unmount() -> None: /// """ -/// Unmount a logical drive +/// Unmount the SD card filesystem. /// """ -STATIC mp_obj_t mod_trezorio_FatFS_unmount(mp_obj_t self) { - // to unmount we have to call mount with the first parameter NULL - FRESULT res = f_mount(NULL, "", 0); - if (res != FR_OK) { - mp_raise_OSError(fresult_to_errno_table[res]); - } +STATIC mp_obj_t mod_trezorio_fatfs_unmount() { + _fatfs_unmount_instance(); return mp_const_none; } -STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_FatFS_unmount_obj, - mod_trezorio_FatFS_unmount); +STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorio_fatfs_unmount_obj, + mod_trezorio_fatfs_unmount); + +/// def is_mounted() -> bool: +/// """ +/// Check if the filesystem is mounted. +/// """ +STATIC mp_obj_t mod_trezorio_fatfs_is_mounted() { + return mp_obj_new_bool(_fatfs_instance_is_mounted()); +} +STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorio_fatfs_is_mounted_obj, + mod_trezorio_fatfs_is_mounted); -/// def mkfs(self) -> None: +/// def mkfs() -> None: /// """ -/// Create a FAT volume +/// Create a FAT volume on the SD card, /// """ -STATIC mp_obj_t mod_trezorio_FatFS_mkfs(mp_obj_t self) { +STATIC mp_obj_t mod_trezorio_fatfs_mkfs() { + if (_fatfs_instance_is_mounted()) { + mp_raise_OSError(MP_EBUSY); + } MKFS_PARM params = {FM_FAT32, 0, 0, 0, 0}; uint8_t working_buf[FF_MAX_SS]; FRESULT res = f_mkfs("", ¶ms, working_buf, sizeof(working_buf)); @@ -513,14 +505,17 @@ STATIC mp_obj_t mod_trezorio_FatFS_mkfs(mp_obj_t self) { } return mp_const_none; } -STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_FatFS_mkfs_obj, - mod_trezorio_FatFS_mkfs); +STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorio_fatfs_mkfs_obj, + mod_trezorio_fatfs_mkfs); -/// def setlabel(self, label: str) -> None: +/// def setlabel(label: str) -> None: /// """ /// Set volume label /// """ -STATIC mp_obj_t mod_trezorio_FatFS_setlabel(mp_obj_t self, mp_obj_t label) { +STATIC mp_obj_t mod_trezorio_fatfs_setlabel(mp_obj_t label) { + if (_fatfs_instance_is_mounted()) { + mp_raise_OSError(MP_EBUSY); + } mp_buffer_info_t _label; mp_get_buffer_raise(label, &_label, MP_BUFFER_READ); FRESULT res = f_setlabel(_label.buf); @@ -529,28 +524,32 @@ STATIC mp_obj_t mod_trezorio_FatFS_setlabel(mp_obj_t self, mp_obj_t label) { } return mp_const_none; } -STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_FatFS_setlabel_obj, - mod_trezorio_FatFS_setlabel); - -STATIC const mp_rom_map_elem_t mod_trezorio_FatFS_locals_dict_table[] = { - {MP_ROM_QSTR(MP_QSTR_open), MP_ROM_PTR(&mod_trezorio_FatFS_open_obj)}, - {MP_ROM_QSTR(MP_QSTR_listdir), MP_ROM_PTR(&mod_trezorio_FatFS_listdir_obj)}, - {MP_ROM_QSTR(MP_QSTR_mkdir), MP_ROM_PTR(&mod_trezorio_FatFS_mkdir_obj)}, - {MP_ROM_QSTR(MP_QSTR_unlink), MP_ROM_PTR(&mod_trezorio_FatFS_unlink_obj)}, - {MP_ROM_QSTR(MP_QSTR_rename), MP_ROM_PTR(&mod_trezorio_FatFS_rename_obj)}, - {MP_ROM_QSTR(MP_QSTR_stat), MP_ROM_PTR(&mod_trezorio_FatFS_stat_obj)}, - {MP_ROM_QSTR(MP_QSTR_mount), MP_ROM_PTR(&mod_trezorio_FatFS_mount_obj)}, - {MP_ROM_QSTR(MP_QSTR_unmount), MP_ROM_PTR(&mod_trezorio_FatFS_unmount_obj)}, - {MP_ROM_QSTR(MP_QSTR_mkfs), MP_ROM_PTR(&mod_trezorio_FatFS_mkfs_obj)}, +STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_fatfs_setlabel_obj, + mod_trezorio_fatfs_setlabel); + +STATIC const mp_rom_map_elem_t mod_trezorio_fatfs_globals_table[] = { + {MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_fatfs)}, + {MP_ROM_QSTR(MP_QSTR_FatFSFile), MP_ROM_PTR(&mod_trezorio_FatFSFile_type)}, + {MP_ROM_QSTR(MP_QSTR_FatFSDir), MP_ROM_PTR(&mod_trezorio_FatFSDir_type)}, + + {MP_ROM_QSTR(MP_QSTR_open), MP_ROM_PTR(&mod_trezorio_fatfs_open_obj)}, + {MP_ROM_QSTR(MP_QSTR_listdir), MP_ROM_PTR(&mod_trezorio_fatfs_listdir_obj)}, + {MP_ROM_QSTR(MP_QSTR_mkdir), MP_ROM_PTR(&mod_trezorio_fatfs_mkdir_obj)}, + {MP_ROM_QSTR(MP_QSTR_unlink), MP_ROM_PTR(&mod_trezorio_fatfs_unlink_obj)}, + {MP_ROM_QSTR(MP_QSTR_rename), MP_ROM_PTR(&mod_trezorio_fatfs_rename_obj)}, + {MP_ROM_QSTR(MP_QSTR_stat), MP_ROM_PTR(&mod_trezorio_fatfs_stat_obj)}, + {MP_ROM_QSTR(MP_QSTR_mount), MP_ROM_PTR(&mod_trezorio_fatfs_mount_obj)}, + {MP_ROM_QSTR(MP_QSTR_unmount), MP_ROM_PTR(&mod_trezorio_fatfs_unmount_obj)}, + {MP_ROM_QSTR(MP_QSTR_is_mounted), + MP_ROM_PTR(&mod_trezorio_fatfs_is_mounted_obj)}, + {MP_ROM_QSTR(MP_QSTR_mkfs), MP_ROM_PTR(&mod_trezorio_fatfs_mkfs_obj)}, {MP_ROM_QSTR(MP_QSTR_setlabel), - MP_ROM_PTR(&mod_trezorio_FatFS_setlabel_obj)}, + MP_ROM_PTR(&mod_trezorio_fatfs_setlabel_obj)}, }; -STATIC MP_DEFINE_CONST_DICT(mod_trezorio_FatFS_locals_dict, - mod_trezorio_FatFS_locals_dict_table); +STATIC MP_DEFINE_CONST_DICT(mod_trezorio_fatfs_globals, + mod_trezorio_fatfs_globals_table); -STATIC const mp_obj_type_t mod_trezorio_FatFS_type = { - {&mp_type_type}, - .name = MP_QSTR_FatFS, - .make_new = mod_trezorio_FatFS_make_new, - .locals_dict = (void *)&mod_trezorio_FatFS_locals_dict, +STATIC const mp_obj_module_t mod_trezorio_fatfs_module = { + .base = {&mp_type_module}, + .globals = (mp_obj_dict_t *)&mod_trezorio_fatfs_globals, }; diff --git a/core/embed/extmod/modtrezorio/modtrezorio-sdcard.h b/core/embed/extmod/modtrezorio/modtrezorio-sdcard.h index 31a089c44..98bbe299b 100644 --- a/core/embed/extmod/modtrezorio/modtrezorio-sdcard.h +++ b/core/embed/extmod/modtrezorio/modtrezorio-sdcard.h @@ -111,6 +111,8 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_sdcard_write_obj, mod_trezorio_sdcard_write); STATIC const mp_rom_map_elem_t mod_trezorio_sdcard_globals_table[] = { + {MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_sdcard)}, + {MP_ROM_QSTR(MP_QSTR_is_present), MP_ROM_PTR(&mod_trezorio_sdcard_is_present_obj)}, {MP_ROM_QSTR(MP_QSTR_power_on), diff --git a/core/embed/extmod/modtrezorio/modtrezorio.c b/core/embed/extmod/modtrezorio/modtrezorio.c index a0595f8ee..286328671 100644 --- a/core/embed/extmod/modtrezorio/modtrezorio.c +++ b/core/embed/extmod/modtrezorio/modtrezorio.c @@ -62,7 +62,7 @@ STATIC const mp_rom_map_elem_t mp_module_trezorio_globals_table[] = { {MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_trezorio)}, - {MP_ROM_QSTR(MP_QSTR_FatFS), MP_ROM_PTR(&mod_trezorio_FatFS_type)}, + {MP_ROM_QSTR(MP_QSTR_fatfs), MP_ROM_PTR(&mod_trezorio_fatfs_module)}, {MP_ROM_QSTR(MP_QSTR_FlashOTP), MP_ROM_PTR(&mod_trezorio_FlashOTP_type)}, diff --git a/core/mocks/generated/trezorio/__init__.pyi b/core/mocks/generated/trezorio/__init__.pyi index 70312c041..b1ab402f9 100644 --- a/core/mocks/generated/trezorio/__init__.pyi +++ b/core/mocks/generated/trezorio/__init__.pyi @@ -1,131 +1,6 @@ from typing import * -# extmod/modtrezorio/modtrezorio-fatfs.h -class FatFSFile: - """ - Class encapsulating file - """ - - def __enter__(self) -> FatFSFile: - """ - Return an open file object - """ - from types import TracebackType - - def __exit__( - self, type: Optional[Type[BaseException]], - value: Optional[BaseException], - traceback: Optional[TracebackType], - ) -> None: - """ - Close an open file object - """ - - def close(self) -> None: - """ - Close an open file object - """ - - def read(self, data: bytearray) -> int: - """ - Read data from the file - """ - - def write(self, data: Union[bytes, bytearray]) -> int: - """ - Write data to the file - """ - - def seek(self, offset: int) -> None: - """ - Move file pointer of the file object - """ - - def truncate(self) -> None: - """ - Truncate the file - """ - - def sync(self) -> None: - """ - Flush cached data of the writing file - """ - - -# extmod/modtrezorio/modtrezorio-fatfs.h -class FatFSDir(Iterator[Tuple[int, str, str]]): - """ - Class encapsulating directory - """ - - def __next__(self) -> Tuple[int, str, str]: - """ - Read an entry in the directory - """ - - -# extmod/modtrezorio/modtrezorio-fatfs.h -class FatFS: - """ - Class encapsulating FAT filesystem - """ - - def __init__(self) -> None: - """ - """ - - def open(self, path: str, flags: str) -> FatFSFile: - """ - Open or create a file - """ - - def listdir(self, path: str) -> FatFSDir: - """ - List a directory (return generator) - """ - - def mkdir(self, path: str, exist_ok: bool=False) -> None: - """ - Create a sub directory - """ - - def unlink(self, path: str) -> None: - """ - Delete an existing file or directory - """ - - def stat(self, path: str) -> Tuple[int, str, str]: - """ - Get file status - """ - - def rename(self, oldpath: str, newpath: str) -> None: - """ - Rename/Move a file or directory - """ - - def mount(self) -> None: - """ - Mount/Unmount a logical drive - """ - - def unmount(self) -> None: - """ - Unmount a logical drive - """ - - def mkfs(self) -> None: - """ - Create a FAT volume - """ - - def setlabel(self, label: str) -> None: - """ - Set volume label - """ - - # extmod/modtrezorio/modtrezorio-flash.h class FlashOTP: """ diff --git a/core/mocks/generated/trezorio/fatfs.pyi b/core/mocks/generated/trezorio/fatfs.pyi new file mode 100644 index 000000000..ffb21e5ad --- /dev/null +++ b/core/mocks/generated/trezorio/fatfs.pyi @@ -0,0 +1,142 @@ +from typing import * + + +# extmod/modtrezorio/modtrezorio-fatfs.h +class FatFSFile: + """ + Class encapsulating file + """ + + def __enter__(self) -> FatFSFile: + """ + Return an open file object + """ + from types import TracebackType + + def __exit__( + self, type: Optional[Type[BaseException]], + value: Optional[BaseException], + traceback: Optional[TracebackType], + ) -> None: + """ + Close an open file object + """ + + def close(self) -> None: + """ + Close an open file object + """ + + def read(self, data: bytearray) -> int: + """ + Read data from the file + """ + + def write(self, data: Union[bytes, bytearray]) -> int: + """ + Write data to the file + """ + + def seek(self, offset: int) -> None: + """ + Move file pointer of the file object + """ + + def truncate(self) -> None: + """ + Truncate the file + """ + + def sync(self) -> None: + """ + Flush cached data of the writing file + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +class FatFSDir(Iterator[Tuple[int, str, str]]): + """ + Class encapsulating directory + """ + + def __next__(self) -> Tuple[int, str, str]: + """ + Read an entry in the directory + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def open(path: str, flags: str) -> FatFSFile: + """ + Open or create a file + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def listdir(path: str) -> FatFSDir: + """ + List a directory (return generator) + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def mkdir(path: str, exist_ok: bool=False) -> None: + """ + Create a sub directory + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def unlink(path: str) -> None: + """ + Delete an existing file or directory + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def stat(path: str) -> Tuple[int, str, str]: + """ + Get file status + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def rename(self, oldpath: str, newpath: str) -> None: + """ + Rename/Move a file or directory + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def mount() -> None: + """ + Mount the SD card filesystem. + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def unmount() -> None: + """ + Unmount the SD card filesystem. + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def is_mounted() -> bool: + """ + Check if the filesystem is mounted. + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def mkfs() -> None: + """ + Create a FAT volume on the SD card, + """ + + +# extmod/modtrezorio/modtrezorio-fatfs.h +def setlabel(label: str) -> None: + """ + Set volume label + """ From c81be584fb8e047a805312c9074f92f609053dd0 Mon Sep 17 00:00:00 2001 From: matejcik Date: Wed, 26 Feb 2020 18:33:43 +0100 Subject: [PATCH 2/5] core/fatfs: ensure functions can only be called on a mounted filesystem ff.c has a lazy-mounting feature, where any filesystem call will mount the volume if it can. This messes with predictability of the mounted state, so all (except mount/unmount/mkfs) Python functions will first check if the fs is mounted. --- .../extmod/modtrezorio/modtrezorio-fatfs.h | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h b/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h index a2bfcebc0..dcbce48bd 100644 --- a/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h +++ b/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h @@ -34,6 +34,13 @@ static FATFS fs_instance; bool _fatfs_instance_is_mounted() { return fs_instance.fs_type != 0; } void _fatfs_unmount_instance() { fs_instance.fs_type = 0; } +#define FATFS_ONLY_MOUNTED \ + { \ + if (!_fatfs_instance_is_mounted()) { \ + mp_raise_OSError(MP_ENODEV); \ + } \ + } + DSTATUS disk_initialize(BYTE pdrv) { return disk_status(pdrv); } DSTATUS disk_status(BYTE pdrv) { @@ -327,6 +334,7 @@ STATIC const mp_obj_type_t mod_trezorio_FatFSDir_type = { /// Open or create a file /// """ STATIC mp_obj_t mod_trezorio_fatfs_open(mp_obj_t path, mp_obj_t flags) { + FATFS_ONLY_MOUNTED; mp_buffer_info_t _path, _flags; mp_get_buffer_raise(path, &_path, MP_BUFFER_READ); mp_get_buffer_raise(flags, &_flags, MP_BUFFER_READ); @@ -369,6 +377,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_fatfs_open_obj, /// List a directory (return generator) /// """ STATIC mp_obj_t mod_trezorio_fatfs_listdir(mp_obj_t path) { + FATFS_ONLY_MOUNTED; mp_buffer_info_t _path; mp_get_buffer_raise(path, &_path, MP_BUFFER_READ); DIR dp; @@ -389,6 +398,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_fatfs_listdir_obj, /// Create a sub directory /// """ STATIC mp_obj_t mod_trezorio_fatfs_mkdir(size_t n_args, const mp_obj_t *args) { + FATFS_ONLY_MOUNTED; mp_buffer_info_t path; mp_get_buffer_raise(args[0], &path, MP_BUFFER_READ); FRESULT res = f_mkdir(path.buf); @@ -409,6 +419,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorio_fatfs_mkdir_obj, 1, 2, /// Delete an existing file or directory /// """ STATIC mp_obj_t mod_trezorio_fatfs_unlink(mp_obj_t path) { + FATFS_ONLY_MOUNTED; mp_buffer_info_t _path; mp_get_buffer_raise(path, &_path, MP_BUFFER_READ); FRESULT res = f_unlink(_path.buf); @@ -425,6 +436,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_fatfs_unlink_obj, /// Get file status /// """ STATIC mp_obj_t mod_trezorio_fatfs_stat(mp_obj_t path) { + FATFS_ONLY_MOUNTED; mp_buffer_info_t _path; mp_get_buffer_raise(path, &_path, MP_BUFFER_READ); FILINFO info; @@ -442,6 +454,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_fatfs_stat_obj, /// Rename/Move a file or directory /// """ STATIC mp_obj_t mod_trezorio_fatfs_rename(mp_obj_t oldpath, mp_obj_t newpath) { + FATFS_ONLY_MOUNTED; mp_buffer_info_t _oldpath, _newpath; mp_get_buffer_raise(oldpath, &_oldpath, MP_BUFFER_READ); mp_get_buffer_raise(newpath, &_newpath, MP_BUFFER_READ); @@ -513,9 +526,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorio_fatfs_mkfs_obj, /// Set volume label /// """ STATIC mp_obj_t mod_trezorio_fatfs_setlabel(mp_obj_t label) { - if (_fatfs_instance_is_mounted()) { - mp_raise_OSError(MP_EBUSY); - } + /* setlabel is marked as only-mounted, because "mounting" in ff.c terms means + having parsed the FAT table, which is of course a prerequisite for setting + label. */ + FATFS_ONLY_MOUNTED; mp_buffer_info_t _label; mp_get_buffer_raise(label, &_label, MP_BUFFER_READ); FRESULT res = f_setlabel(_label.buf); From b24411b900dc0d97e4a160d0b8f7208e7001fb40 Mon Sep 17 00:00:00 2001 From: matejcik Date: Wed, 26 Feb 2020 18:39:45 +0100 Subject: [PATCH 3/5] core/sdcard: unmount instance when powering off sdcard --- core/embed/extmod/modtrezorio/modtrezorio-sdcard.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/embed/extmod/modtrezorio/modtrezorio-sdcard.h b/core/embed/extmod/modtrezorio/modtrezorio-sdcard.h index 98bbe299b..9dd92be22 100644 --- a/core/embed/extmod/modtrezorio/modtrezorio-sdcard.h +++ b/core/embed/extmod/modtrezorio/modtrezorio-sdcard.h @@ -56,6 +56,8 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorio_sdcard_power_on_obj, /// Power off the SD card interface. /// """ STATIC mp_obj_t mod_trezorio_sdcard_power_off() { + /* XXX should this call happen inside sdcard_power_off()? */ + _fatfs_unmount_instance(); sdcard_power_off(); return mp_const_none; } From 9ab84d2455c0ddb37749fb17f602f2800c4c40d8 Mon Sep 17 00:00:00 2001 From: matejcik Date: Wed, 26 Feb 2020 18:41:00 +0100 Subject: [PATCH 4/5] core/tests: thoroughly test modified APIs --- .../extmod/modtrezorio/modtrezorio-fatfs.h | 2 +- core/mocks/generated/trezorio/fatfs.pyi | 2 +- core/tests/test_trezor.io.fatfs.py | 200 ++++++++++++++---- 3 files changed, 157 insertions(+), 47 deletions(-) diff --git a/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h b/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h index dcbce48bd..fa2c4dbc3 100644 --- a/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h +++ b/core/embed/extmod/modtrezorio/modtrezorio-fatfs.h @@ -449,7 +449,7 @@ STATIC mp_obj_t mod_trezorio_fatfs_stat(mp_obj_t path) { STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorio_fatfs_stat_obj, mod_trezorio_fatfs_stat); -/// def rename(self, oldpath: str, newpath: str) -> None: +/// def rename(oldpath: str, newpath: str) -> None: /// """ /// Rename/Move a file or directory /// """ diff --git a/core/mocks/generated/trezorio/fatfs.pyi b/core/mocks/generated/trezorio/fatfs.pyi index ffb21e5ad..d8bc6fd82 100644 --- a/core/mocks/generated/trezorio/fatfs.pyi +++ b/core/mocks/generated/trezorio/fatfs.pyi @@ -101,7 +101,7 @@ def stat(path: str) -> Tuple[int, str, str]: # extmod/modtrezorio/modtrezorio-fatfs.h -def rename(self, oldpath: str, newpath: str) -> None: +def rename(oldpath: str, newpath: str) -> None: """ Rename/Move a file or directory """ diff --git a/core/tests/test_trezor.io.fatfs.py b/core/tests/test_trezor.io.fatfs.py index 1e2a7fbf7..269f10502 100644 --- a/core/tests/test_trezor.io.fatfs.py +++ b/core/tests/test_trezor.io.fatfs.py @@ -1,19 +1,17 @@ from common import * -from trezor import io +from trezorio import sdcard, fatfs class TestTrezorIoFatfs(unittest.TestCase): - def setUp(self): - io.sdcard.power_on() - self.fs = io.FatFS() - self.fs.mkfs() - self.fs.mount() + sdcard.power_on() + fatfs.mkfs() + fatfs.mount() def tearDown(self): - self.fs.unmount() - io.sdcard.power_off() + fatfs.unmount() + sdcard.power_off() def _filename(self, suffix=""): return "FILE%s.TXT" % suffix @@ -26,78 +24,80 @@ class TestTrezorIoFatfs(unittest.TestCase): pass def test_mkdir(self): - self.fs.mkdir("/%s" % self._dirname()) - s = self.fs.stat("/%s" % self._dirname()) + fatfs.mkdir("/%s" % self._dirname()) + s = fatfs.stat("/%s" % self._dirname()) self.assertEqual(s, (0, "---d-", self._dirname())) def test_listdir(self): - self.fs.mkdir("/%s" % self._dirname()) - with self.fs.open("/%s" % self._filename(), "w") as f: + fatfs.mkdir("/%s" % self._dirname()) + with fatfs.open("/%s" % self._filename(), "w") as f: f.write(bytearray(b"test")) - with self.fs.open("/%s/%s" % (self._dirname(), self._filename("2")), "w") as f: + with fatfs.open("/%s/%s" % (self._dirname(), self._filename("2")), "w") as f: f.write(bytearray(b"testtest")) - l = [e for e in self.fs.listdir("/")] - self.assertEqual(l, [(0, "---d-", self._dirname()), (4, "----a", self._filename())]) - l = [e for e in self.fs.listdir("/%s" % self._dirname())] + l = [e for e in fatfs.listdir("/")] + self.assertEqual( + l, [(0, "---d-", self._dirname()), (4, "----a", self._filename())] + ) + l = [e for e in fatfs.listdir("/%s" % self._dirname())] self.assertEqual(l, [(8, "----a", self._filename("2"))]) def test_unlink(self): - self.fs.mkdir("/%s" % self._dirname()) - with self.fs.open("/%s" % self._filename(), "w") as f: + fatfs.mkdir("/%s" % self._dirname()) + with fatfs.open("/%s" % self._filename(), "w") as f: f.write(bytearray(b"test")) - s = self.fs.stat("/%s" % self._dirname()) + s = fatfs.stat("/%s" % self._dirname()) self.assertEqual(s, (0, "---d-", self._dirname())) - s = self.fs.stat("/%s" % self._filename()) + s = fatfs.stat("/%s" % self._filename()) self.assertEqual(s, (4, "----a", self._filename())) - self.fs.unlink("/%s" % self._dirname()) - self.fs.unlink("/%s" % self._filename()) + fatfs.unlink("/%s" % self._dirname()) + fatfs.unlink("/%s" % self._filename()) with self.assertRaises(OSError): - self.fs.stat("/%s" % self._dirname()) + fatfs.stat("/%s" % self._dirname()) with self.assertRaises(OSError): - self.assertRaises(self.fs.stat("/%s" % self._filename())) + self.assertRaises(fatfs.stat("/%s" % self._filename())) def test_rename(self): - self.fs.mkdir("/%s" % self._dirname()) - with self.fs.open("/%s" % self._filename(), "w") as f: + fatfs.mkdir("/%s" % self._dirname()) + with fatfs.open("/%s" % self._filename(), "w") as f: f.write(bytearray(b"test")) - s = self.fs.stat("/%s" % self._dirname()) + s = fatfs.stat("/%s" % self._dirname()) self.assertEqual(s, (0, "---d-", self._dirname())) - s = self.fs.stat("/%s" % self._filename()) + s = fatfs.stat("/%s" % self._filename()) self.assertEqual(s, (4, "----a", self._filename())) - self.fs.rename("/%s" % self._dirname(), "/%s" % self._dirname("2")) - self.fs.rename("/%s" % self._filename(), "/%s" % self._filename("2")) + fatfs.rename("/%s" % self._dirname(), "/%s" % self._dirname("2")) + fatfs.rename("/%s" % self._filename(), "/%s" % self._filename("2")) with self.assertRaises(OSError): - self.fs.stat("/%s" % self._dirname()) + fatfs.stat("/%s" % self._dirname()) with self.assertRaises(OSError): - self.assertRaises(self.fs.stat("/%s" % self._filename())) - s = self.fs.stat("/%s" % self._dirname("2")) + self.assertRaises(fatfs.stat("/%s" % self._filename())) + s = fatfs.stat("/%s" % self._dirname("2")) self.assertEqual(s, (0, "---d-", self._dirname("2"))) - s = self.fs.stat("/%s" % self._filename("2")) + s = fatfs.stat("/%s" % self._filename("2")) self.assertEqual(s, (4, "----a", self._filename("2"))) def test_open_rw(self): - with self.fs.open("/%s" % self._filename(), "w") as f: + with fatfs.open("/%s" % self._filename(), "w") as f: f.write(bytearray(b"test")) - with self.fs.open("/%s" % self._filename(), "r") as f: + with fatfs.open("/%s" % self._filename(), "r") as f: b = bytearray(100) r = f.read(b) self.assertEqual(r, 4) self.assertEqual(bytes(b[:4]), b"test") def test_open_a(self): - with self.fs.open("/%s" % self._filename(), "w") as f: + with fatfs.open("/%s" % self._filename(), "w") as f: f.write(bytearray(b"test" * 200)) - with self.fs.open("/%s" % self._filename(), "a") as f: + with fatfs.open("/%s" % self._filename(), "a") as f: f.seek(800) f.write(bytearray(b"TEST" * 200)) - with self.fs.open("/%s" % self._filename(), "r") as f: + with fatfs.open("/%s" % self._filename(), "r") as f: b = bytearray(2000) r = f.read(b) self.assertEqual(r, 1600) self.assertEqual(bytes(b[:1600]), b"test" * 200 + b"TEST" * 200) def test_seek(self): - with self.fs.open("/%s" % self._filename(), "w+") as f: + with fatfs.open("/%s" % self._filename(), "w+") as f: f.write(bytearray(b"test" * 10)) f.seek(2) b = bytearray(8) @@ -106,19 +106,18 @@ class TestTrezorIoFatfs(unittest.TestCase): self.assertEqual(bytes(b[:8]), b"sttestte") def test_truncate(self): - with self.fs.open("/%s" % self._filename(), "w") as f: + with fatfs.open("/%s" % self._filename(), "w") as f: f.write(bytearray(b"test" * 100)) - s = self.fs.stat("/%s" % self._filename()) + s = fatfs.stat("/%s" % self._filename()) self.assertEqual(s, (400, "----a", self._filename())) - with self.fs.open("/%s" % self._filename(), "a") as f: + with fatfs.open("/%s" % self._filename(), "a") as f: f.seek(111) f.truncate() - s = self.fs.stat("/%s" % self._filename()) + s = fatfs.stat("/%s" % self._filename()) self.assertEqual(s, (111, "----a", self._filename())) class TestTrezorIoFatfsLfn(TestTrezorIoFatfs): - def _filename(self, suffix=""): return "reallylongfilename%s.textfile" % suffix @@ -126,5 +125,116 @@ class TestTrezorIoFatfsLfn(TestTrezorIoFatfs): return "reallylongdirname%s" % suffix +class TestTrezorIoFatfsMounting(unittest.TestCase): + MOUNTED_METHODS = [ + ("open", ("hello.txt", "w")), + ("listdir", ("",)), + ("mkdir", ("testdir",)), + ("unlink", ("hello.txt",)), + ("stat", ("testdir",)), + ("rename", ("testdir", "newdir")), + ("setlabel", ("label",)), + ] + UNMOUNTED_METHODS = [ + ("mkfs", ()), + ] + OTHER = { + "__name__", + "__class__", + "mount", + "unmount", + "is_mounted", + "FatFSFile", + "FatFSDir", + } + + def setUp(self): + sdcard.power_on() + + def tearDown(self): + sdcard.power_off() + + def test_mount_unmount(self): + fatfs.mkfs() + + self.assertFalse(fatfs.is_mounted()) + fatfs.mount() + self.assertTrue(fatfs.is_mounted()) + fatfs.mount() + self.assertTrue(fatfs.is_mounted()) + fatfs.unmount() + self.assertFalse(fatfs.is_mounted()) + + def test_no_filesystem(self): + # trash FAT table + sdcard.write(0, bytes([0xFF] * sdcard.BLOCK_SIZE)) + + self.assertFalse(fatfs.is_mounted()) + try: + fatfs.mount() + self.fail("should have raised") + except OSError as e: + self.assertEqual(e.args[0], 19) # ENODEV + self.assertFalse(fatfs.is_mounted()) + + def test_exhaustive(self): + all_symbols = ( + set(name for name, call in (self.MOUNTED_METHODS + self.UNMOUNTED_METHODS)) + | self.OTHER + ) + self.assertEqual(set(dir(fatfs)), all_symbols) + + def test_mounted(self): + fatfs.mkfs() + fatfs.mount() + self.assertTrue(fatfs.is_mounted()) + + for name, call in self.MOUNTED_METHODS: + function = getattr(fatfs, name) + function(*call) + + for name, call in self.UNMOUNTED_METHODS: + function = getattr(fatfs, name) + try: + function(*call) + self.fail("should have raised") + except OSError as e: + self.assertEqual(e.args[0], 16) # EBUSY + + def test_unmounted(self): + fatfs.unmount() + fatfs.mkfs() + self.assertFalse(fatfs.is_mounted()) + + for name, call in self.UNMOUNTED_METHODS: + function = getattr(fatfs, name) + function(*call) + self.assertFalse(fatfs.is_mounted()) + + for name, call in self.MOUNTED_METHODS: + function = getattr(fatfs, name) + try: + function(*call) + self.fail("should have raised") + except OSError as e: + self.assertEqual(e.args[0], 19) # ENODEV + + +class TestTrezorIoFatfsAndSdcard(unittest.TestCase): + def test_sd_power(self): + sdcard.power_off() + self.assertFalse(fatfs.is_mounted()) + self.assertRaises(OSError, fatfs.mount) + + sdcard.power_on() + self.assertFalse(fatfs.is_mounted()) + fatfs.mkfs() + fatfs.mount() + self.assertTrue(fatfs.is_mounted()) + + sdcard.power_off() + self.assertFalse(fatfs.is_mounted()) + + if __name__ == "__main__": unittest.main() From 18ac4fc9ca4150c746f5a200d12f8e63bf559c12 Mon Sep 17 00:00:00 2001 From: matejcik Date: Wed, 26 Feb 2020 19:25:22 +0100 Subject: [PATCH 5/5] core: update Python facing APIs --- core/src/apps/common/sdcard.py | 23 +++++--- core/src/apps/debug/__init__.py | 3 +- core/src/storage/sd_salt.py | 83 ++++++++++++++-------------- core/src/trezor/__init__.py | 2 + core/src/trezor/sdcard.py | 26 ++++++--- core/tests/test_trezor.sdcard.py | 94 +++++++++++++++----------------- 6 files changed, 120 insertions(+), 111 deletions(-) diff --git a/core/src/apps/common/sdcard.py b/core/src/apps/common/sdcard.py index 56a0dd2d4..8c8984bef 100644 --- a/core/src/apps/common/sdcard.py +++ b/core/src/apps/common/sdcard.py @@ -1,6 +1,6 @@ import storage.sd_salt from storage.sd_salt import SD_CARD_HOT_SWAPPABLE -from trezor import sdcard, ui, wire +from trezor import fatfs, sdcard, ui, wire from trezor.ui.text import Text from apps.common.confirm import confirm, hold_to_confirm @@ -91,8 +91,8 @@ async def ensure_sdcard( while True: try: - with sdcard.get_filesystem(mounted=False) as fs: - fs.mount() + with sdcard.filesystem(mounted=False): + fatfs.mount() # Mount succeeded, filesystem is OK return except OSError: @@ -103,13 +103,18 @@ async def ensure_sdcard( raise SdCardUnavailable("SD card not formatted.") try: - with sdcard.get_filesystem(mounted=False) as fs: - fs.mkfs() - # mkfs succeeded. Re-run loop to retry mounting. - continue + with sdcard.filesystem(mounted=False): + fatfs.mkfs() + fatfs.mount() + fatfs.setlabel("TREZOR") + # mkfs and mount succeeded + return except OSError: - if not await sd_problem_dialog(ctx): - raise SdCardUnavailable("Problem formatting SD card.") + pass + + # allow retry if we get as far as here + if not await sd_problem_dialog(ctx): + raise SdCardUnavailable("Problem formatting SD card.") async def request_sd_salt( diff --git a/core/src/apps/debug/__init__.py b/core/src/apps/debug/__init__.py index af2122e47..74b22600e 100644 --- a/core/src/apps/debug/__init__.py +++ b/core/src/apps/debug/__init__.py @@ -145,8 +145,7 @@ if __debug__: try: io.sdcard.power_on() if msg.format: - fs = io.FatFS() - fs.mkfs() + io.fatfs.mkfs() else: # trash first 1 MB of data to destroy the FAT filesystem assert io.sdcard.capacity() >= 1024 * 1024 diff --git a/core/src/storage/sd_salt.py b/core/src/storage/sd_salt.py index 654d57ec0..0c8de5271 100644 --- a/core/src/storage/sd_salt.py +++ b/core/src/storage/sd_salt.py @@ -1,13 +1,13 @@ from micropython import const import storage.device +from trezor import fatfs from trezor.crypto import hmac from trezor.crypto.hashlib import sha256 -from trezor.sdcard import get_filesystem +from trezor.sdcard import with_filesystem from trezor.utils import consteq if False: - from trezor import io from typing import Optional, TypeVar, Callable T = TypeVar("T", bound=Callable) @@ -38,10 +38,11 @@ def _get_salt_path(new: bool = False) -> str: return "{}/salt{}".format(_get_device_dir(), ".new" if new else "") -def _load_salt(fs: io.FatFS, auth_key: bytes, path: str) -> Optional[bytearray]: +@with_filesystem +def _load_salt(auth_key: bytes, path: str) -> Optional[bytearray]: # Load the salt file if it exists. try: - with fs.open(path, "r") as f: + with fatfs.open(path, "r") as f: salt = bytearray(SD_SALT_LEN_BYTES) stored_tag = bytearray(SD_SALT_AUTH_TAG_LEN_BYTES) f.read(salt) @@ -57,6 +58,7 @@ def _load_salt(fs: io.FatFS, auth_key: bytes, path: str) -> Optional[bytearray]: return salt +@with_filesystem def load_sd_salt() -> Optional[bytearray]: salt_auth_key = storage.device.get_sd_salt_auth_key() if salt_auth_key is None: @@ -65,55 +67,54 @@ def load_sd_salt() -> Optional[bytearray]: salt_path = _get_salt_path() new_salt_path = _get_salt_path(new=True) - with get_filesystem() as fs: - salt = _load_salt(fs, salt_auth_key, salt_path) - if salt is not None: - return salt - - # Check if there is a new salt. - salt = _load_salt(fs, salt_auth_key, new_salt_path) - if salt is None: - # No valid salt file on this SD card. - raise WrongSdCard - - # Normal salt file does not exist, but new salt file exists. That means that - # SD salt regeneration was interrupted earlier. Bring into consistent state. - # TODO Possibly overwrite salt file with random data. - try: - fs.unlink(salt_path) - except OSError: - pass - - # fs.rename can fail with a write error, which falls through as an OSError. - # This should be handled in calling code, by allowing the user to retry. - fs.rename(new_salt_path, salt_path) + salt = _load_salt(salt_auth_key, salt_path) + if salt is not None: return salt + # Check if there is a new salt. + salt = _load_salt(salt_auth_key, new_salt_path) + if salt is None: + # No valid salt file on this SD card. + raise WrongSdCard + # Normal salt file does not exist, but new salt file exists. That means that + # SD salt regeneration was interrupted earlier. Bring into consistent state. + # TODO Possibly overwrite salt file with random data. + try: + fatfs.unlink(salt_path) + except OSError: + pass + + # fatfs.rename can fail with a write error, which falls through as an OSError. + # This should be handled in calling code, by allowing the user to retry. + fatfs.rename(new_salt_path, salt_path) + return salt + + +@with_filesystem def set_sd_salt(salt: bytes, salt_tag: bytes, stage: bool = False) -> None: salt_path = _get_salt_path(stage) - with get_filesystem() as fs: - fs.mkdir("/trezor", True) - fs.mkdir(_get_device_dir(), True) - with fs.open(salt_path, "w") as f: - f.write(salt) - f.write(salt_tag) + fatfs.mkdir("/trezor", True) + fatfs.mkdir(_get_device_dir(), True) + with fatfs.open(salt_path, "w") as f: + f.write(salt) + f.write(salt_tag) +@with_filesystem def commit_sd_salt() -> None: salt_path = _get_salt_path(new=False) new_salt_path = _get_salt_path(new=True) - with get_filesystem() as fs: - try: - fs.unlink(salt_path) - except OSError: - pass - fs.rename(new_salt_path, salt_path) + try: + fatfs.unlink(salt_path) + except OSError: + pass + fatfs.rename(new_salt_path, salt_path) +@with_filesystem def remove_sd_salt() -> None: salt_path = _get_salt_path() - with get_filesystem() as fs: - # TODO Possibly overwrite salt file with random data. - fs.unlink(salt_path) + # TODO Possibly overwrite salt file with random data. + fatfs.unlink(salt_path) diff --git a/core/src/trezor/__init__.py b/core/src/trezor/__init__.py index 80c7ec815..25c858cde 100644 --- a/core/src/trezor/__init__.py +++ b/core/src/trezor/__init__.py @@ -1,2 +1,4 @@ import trezorconfig as config # noqa: F401 import trezorio as io # noqa: F401 + +fatfs = io.fatfs diff --git a/core/src/trezor/sdcard.py b/core/src/trezor/sdcard.py index 0f8d340d8..06c6c39e9 100644 --- a/core/src/trezor/sdcard.py +++ b/core/src/trezor/sdcard.py @@ -1,14 +1,15 @@ -from trezorio import FatFS, sdcard +from trezorio import fatfs, sdcard if False: - from typing import Any, Optional + from typing import Any, Callable, Optional, TypeVar + + T = TypeVar("T", bound=Callable) class FilesystemWrapper: _INSTANCE = None # type: Optional[FilesystemWrapper] def __init__(self, mounted: bool) -> None: - self.fs = FatFS() self.mounted = mounted self.counter = 0 @@ -21,20 +22,18 @@ class FilesystemWrapper: return cls._INSTANCE def _deinit_instance(self) -> None: - if self.mounted: - self.fs.unmount() + fatfs.unmount() sdcard.power_off() FilesystemWrapper._INSTANCE = None - def __enter__(self) -> "FatFS": + def __enter__(self) -> None: try: if self.counter <= 0: self.counter = 0 sdcard.power_on() if self.mounted: - self.fs.mount() + fatfs.mount() self.counter += 1 - return self.fs except Exception: self._deinit_instance() raise @@ -46,8 +45,17 @@ class FilesystemWrapper: self._deinit_instance() -def get_filesystem(mounted: bool = True) -> FilesystemWrapper: +def filesystem(mounted: bool = True) -> FilesystemWrapper: return FilesystemWrapper.get_instance(mounted=mounted) +def with_filesystem(func: T) -> T: + def wrapped_func(*args, **kwargs) -> Any: # type: ignore + with filesystem(): + return func(*args, **kwargs) + + return wrapped_func # type: ignore + + is_present = sdcard.is_present +capacity = sdcard.capacity diff --git a/core/tests/test_trezor.sdcard.py b/core/tests/test_trezor.sdcard.py index 98707a38c..f70853ac5 100644 --- a/core/tests/test_trezor.sdcard.py +++ b/core/tests/test_trezor.sdcard.py @@ -1,75 +1,68 @@ from common import * -from trezor import io, sdcard +from trezor import io, fatfs, sdcard class TestTrezorSdcard(unittest.TestCase): def test_power(self): - # io.sdcard.capacity() will return 0 if the card is not powered, + # sdcard.capacity() will return 0 if the card is not powered, # non-zero value otherwise - self.assertEqual(io.sdcard.capacity(), 0) - with sdcard.get_filesystem(mounted=False): - self.assertTrue(io.sdcard.capacity() > 0) - self.assertEqual(io.sdcard.capacity(), 0) + self.assertEqual(sdcard.capacity(), 0) + with sdcard.filesystem(mounted=False): + self.assertTrue(sdcard.capacity() > 0) + self.assertEqual(sdcard.capacity(), 0) def test_nomount(self): - with sdcard.get_filesystem(mounted=False) as fs: - with self.assertRaises(OSError): - fs.listdir("/") + with sdcard.filesystem(mounted=False): + self.assertFalse(fatfs.is_mounted()) def test_mount(self): # set up a filesystem first - with sdcard.get_filesystem(mounted=False) as fs: - fs.mkfs() + with sdcard.filesystem(mounted=False): + fatfs.mkfs() - with sdcard.get_filesystem() as fs: - # the following should succeed - fs.listdir("/") + with sdcard.filesystem(): + self.assertTrue(fatfs.is_mounted()) - # filesystem should not be available - with self.assertRaises(OSError): - fs.listdir("/") + self.assertFalse(fatfs.is_mounted()) def test_nesting(self): # set up a filesystem first - with sdcard.get_filesystem(mounted=False) as fs: - fs.mkfs() - - self.assertEqual(io.sdcard.capacity(), 0) - with sdcard.get_filesystem() as fs_a: - self.assertTrue(io.sdcard.capacity() > 0) - with sdcard.get_filesystem() as fs_b: - self.assertTrue(io.sdcard.capacity() > 0) - self.assertIs(fs_a, fs_b) - fs_b.listdir("/") - self.assertTrue(io.sdcard.capacity() > 0) - # filesystem should still be mounted - fs_a.listdir("/") - - self.assertEqual(io.sdcard.capacity(), 0) - # filesystem should not be available - with self.assertRaises(OSError): - fs_a.listdir("/") + with sdcard.filesystem(mounted=False): + fatfs.mkfs() + + self.assertEqual(sdcard.capacity(), 0) + with sdcard.filesystem(): + self.assertTrue(sdcard.capacity() > 0) + self.assertTrue(fatfs.is_mounted()) + with sdcard.filesystem(): + self.assertTrue(sdcard.capacity() > 0) + self.assertTrue(fatfs.is_mounted()) + + self.assertTrue(sdcard.capacity() > 0) + self.assertTrue(fatfs.is_mounted()) + + self.assertEqual(sdcard.capacity(), 0) + self.assertFalse(fatfs.is_mounted()) def test_mount_nomount(self): with self.assertRaises(RuntimeError): - with sdcard.get_filesystem(mounted=True): - with sdcard.get_filesystem(mounted=False): + with sdcard.filesystem(mounted=True): + with sdcard.filesystem(mounted=False): pass with self.assertRaises(RuntimeError): - with sdcard.get_filesystem(mounted=False): - with sdcard.get_filesystem(mounted=True): + with sdcard.filesystem(mounted=False): + with sdcard.filesystem(mounted=True): pass def test_failed_mount(self): # set up a filesystem first - with sdcard.get_filesystem(mounted=False) as fs: - fs.mkfs() + with sdcard.filesystem(mounted=False): + fatfs.mkfs() - with sdcard.get_filesystem() as fs: - # the following should succeed - fs.listdir("/") + with sdcard.filesystem(): + self.assertTrue(fatfs.is_mounted()) # trash filesystem io.sdcard.power_on() @@ -78,17 +71,18 @@ class TestTrezorSdcard(unittest.TestCase): # mounting should now fail with self.assertRaises(OSError): - with sdcard.get_filesystem() as fs: + with sdcard.filesystem(): pass + self.assertFalse(fatfs.is_mounted()) + # it should be possible to create an unmounted instance - with sdcard.get_filesystem(mounted=False) as fs: - fs.mkfs() + with sdcard.filesystem(mounted=False): + fatfs.mkfs() # mounting should now succeed - with sdcard.get_filesystem() as fs: - fs.listdir("/") - + with sdcard.filesystem(): + self.assertTrue(fatfs.is_mounted()) if __name__ == "__main__":