From 1a0596a2fbb68c1285ec59606d03a610ee94ca47 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Tue, 19 Feb 2019 14:45:45 -0800 Subject: [PATCH 1/3] Add option to disable the concurrent write protection This allows writing to the filesystem from the host computer and CircuitPython by increasing the risk of filesystem corruption. --- extmod/vfs_fat.h | 2 ++ extmod/vfs_fat_file.c | 3 ++- lib/tinyusb | 2 +- main.c | 7 +++--- shared-bindings/storage/__init__.c | 23 +++++++++++++------ shared-bindings/storage/__init__.h | 2 +- shared-module/storage/__init__.c | 5 +++-- supervisor/filesystem.h | 9 +++++++- supervisor/flash.h | 1 - supervisor/shared/filesystem.c | 32 +++++++++++++++++++++++++-- supervisor/shared/flash.c | 20 ----------------- supervisor/shared/usb/usb_msc_flash.c | 4 ++-- 12 files changed, 69 insertions(+), 41 deletions(-) diff --git a/extmod/vfs_fat.h b/extmod/vfs_fat.h index dd5e5ffcb2..08b03d0d18 100644 --- a/extmod/vfs_fat.h +++ b/extmod/vfs_fat.h @@ -38,6 +38,8 @@ #define FSUSER_NO_FILESYSTEM (0x0008) // the block device has no filesystem on it // Device is writable over USB and read-only to MicroPython. #define FSUSER_USB_WRITABLE (0x0010) +// Bit set when the above flag is checked before opening a file for write. +#define FSUSER_CONCURRENT_WRITE_PROTECTED (0x0020) typedef struct _fs_user_mount_t { mp_obj_base_t base; diff --git a/extmod/vfs_fat_file.c b/extmod/vfs_fat_file.c index 690073ce48..6aad1884cb 100644 --- a/extmod/vfs_fat_file.c +++ b/extmod/vfs_fat_file.c @@ -34,6 +34,7 @@ #include "py/mperrno.h" #include "lib/oofatfs/ff.h" #include "extmod/vfs_fat.h" +#include "supervisor/filesystem.h" // this table converts from FRESULT to POSIX errno const byte fresult_to_errno_table[20] = { @@ -187,7 +188,7 @@ STATIC mp_obj_t file_open(fs_user_mount_t *vfs, const mp_obj_type_t *type, mp_ar } } assert(vfs != NULL); - if ((vfs->flags & FSUSER_USB_WRITABLE) != 0 && (mode & FA_WRITE) != 0) { + if ((mode & FA_WRITE) != 0 && !filesystem_is_writable_by_python(vfs)) { mp_raise_OSError(MP_EROFS); } diff --git a/lib/tinyusb b/lib/tinyusb index 29b49199be..55874813f8 160000 --- a/lib/tinyusb +++ b/lib/tinyusb @@ -1 +1 @@ -Subproject commit 29b49199beb8e9b5fead83e5cd36105f8746f1d7 +Subproject commit 55874813f82157b7509729b1a0c66e68f86e2d07 diff --git a/main.c b/main.c index 68ee66d02a..f0913db6ee 100755 --- a/main.c +++ b/main.c @@ -323,12 +323,12 @@ void __attribute__ ((noinline)) run_boot_py(safe_mode_t safe_mode) { mp_hal_delay_ms(1500); // USB isn't up, so we can write the file. - filesystem_writable_by_python(true); + filesystem_set_internal_writable_by_usb(false); f_open(fs, boot_output_file, CIRCUITPY_BOOT_OUTPUT_FILE, FA_WRITE | FA_CREATE_ALWAYS); // Switch the filesystem back to non-writable by Python now instead of later, // since boot.py might change it back to writable. - filesystem_writable_by_python(false); + filesystem_set_internal_writable_by_usb(true); // Write version info to boot_out.txt. mp_hal_stdout_tx_str(MICROPY_FULL_VERSION_INFO); @@ -416,7 +416,8 @@ int __attribute__((used)) main(void) { // By default our internal flash is readonly to local python code and // writable over USB. Set it here so that boot.py can change it. - filesystem_writable_by_python(false); + filesystem_set_internal_concurrent_write_protection(true); + filesystem_set_internal_writable_by_usb(true); run_boot_py(safe_mode); diff --git a/shared-bindings/storage/__init__.c b/shared-bindings/storage/__init__.c index 3484b9af82..d7af48df00 100644 --- a/shared-bindings/storage/__init__.c +++ b/shared-bindings/storage/__init__.c @@ -48,16 +48,18 @@ //| directly. //| -//| .. function:: mount(filesystem, mount_path, \*, readonly=False) +//| .. function:: mount(filesystem, mount_path, \*, readonly=False, disable_concurrent_write_protection=False) //| //| Mounts the given filesystem object at the given path. //| //| This is the CircuitPython analog to the UNIX ``mount`` command. //| +//| :param bool readonly: True when the filesystem should be readonly to CircuitPython. +//| mp_obj_t storage_mount(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { enum { ARG_readonly }; static const mp_arg_t allowed_args[] = { - { MP_QSTR_readonly, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_false} }, + { MP_QSTR_readonly, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} }, }; // parse args @@ -77,7 +79,7 @@ mp_obj_t storage_mount(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_arg mp_raise_ValueError(translate("filesystem must provide mount method")); } - common_hal_storage_mount(vfs_obj, mnt_str, mp_obj_is_true(args[ARG_readonly].u_obj)); + common_hal_storage_mount(vfs_obj, mnt_str, args[ARG_readonly].u_bool); return mp_const_none; } @@ -101,14 +103,21 @@ mp_obj_t storage_umount(mp_obj_t mnt_in) { } MP_DEFINE_CONST_FUN_OBJ_1(storage_umount_obj, storage_umount); -//| .. function:: remount(mount_path, readonly=False) +//| .. function:: remount(mount_path, readonly=False, *, disable_concurrent_write_protection=False) //| //| Remounts the given path with new parameters. //| +//| :param bool readonly: True when the filesystem should be readonly to CircuitPython. +//| :param bool disable_concurrent_write_protection: When True, the check that makes sure the +//| underlying filesystem data is written by one computer is disabled. Disabling the protection +//| allows CircuitPython and a host to write to the same filesystem with the risk that the +//| filesystem will be corrupted. +//| mp_obj_t storage_remount(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { - enum { ARG_readonly }; + enum { ARG_readonly, ARG_disable_concurrent_write_protection }; static const mp_arg_t allowed_args[] = { - { MP_QSTR_readonly, MP_ARG_BOOL | MP_ARG_REQUIRED, {.u_bool = false} }, + { MP_QSTR_readonly, MP_ARG_BOOL, {.u_bool = false} }, + { MP_QSTR_disable_concurrent_write_protection, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} }, }; // get the mount point @@ -118,7 +127,7 @@ mp_obj_t storage_remount(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_a mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); - common_hal_storage_remount(mnt_str, args[ARG_readonly].u_bool); + common_hal_storage_remount(mnt_str, args[ARG_readonly].u_bool, args[ARG_disable_concurrent_write_protection].u_bool); return mp_const_none; } diff --git a/shared-bindings/storage/__init__.h b/shared-bindings/storage/__init__.h index 6a0cb9fc71..7851b9e292 100644 --- a/shared-bindings/storage/__init__.h +++ b/shared-bindings/storage/__init__.h @@ -33,7 +33,7 @@ void common_hal_storage_mount(mp_obj_t vfs_obj, const char* path, bool readonly); void common_hal_storage_umount_path(const char* path); void common_hal_storage_umount_object(mp_obj_t vfs_obj); -void common_hal_storage_remount(const char* path, bool readonly); +void common_hal_storage_remount(const char* path, bool readonly, bool disable_concurrent_write_protection); mp_obj_t common_hal_storage_getmount(const char* path); void common_hal_storage_erase_filesystem(void); diff --git a/shared-module/storage/__init__.c b/shared-module/storage/__init__.c index f09edd7858..215e1356a3 100644 --- a/shared-module/storage/__init__.c +++ b/shared-module/storage/__init__.c @@ -143,7 +143,7 @@ mp_obj_t common_hal_storage_getmount(const char *mount_path) { return storage_object_from_path(mount_path); } -void common_hal_storage_remount(const char *mount_path, bool readonly) { +void common_hal_storage_remount(const char *mount_path, bool readonly, bool disable_concurrent_write_protection) { if (strcmp(mount_path, "/") != 0) { mp_raise_OSError(MP_EINVAL); } @@ -156,7 +156,8 @@ void common_hal_storage_remount(const char *mount_path, bool readonly) { } #endif - supervisor_flash_set_usb_writable(readonly); + filesystem_set_internal_writable_by_usb(readonly); + filesystem_set_internal_concurrent_write_protection(!disable_concurrent_write_protection); } void common_hal_storage_erase_filesystem(void) { diff --git a/supervisor/filesystem.h b/supervisor/filesystem.h index eb9e65c091..76f235f47d 100644 --- a/supervisor/filesystem.h +++ b/supervisor/filesystem.h @@ -29,9 +29,16 @@ #include +#include "extmod/vfs_fat.h" + void filesystem_init(bool create_allowed, bool force_create); void filesystem_flush(void); -void filesystem_writable_by_python(bool writable); bool filesystem_present(void); +void filesystem_set_internal_writable_by_usb(bool usb_writable); +void filesystem_set_internal_concurrent_write_protection(bool concurrent_write_protection); +void filesystem_set_writable_by_usb(fs_user_mount_t *vfs, bool usb_writable); +void filesystem_set_concurrent_write_protection(fs_user_mount_t *vfs, bool concurrent_write_protection); +bool filesystem_is_writable_by_python(fs_user_mount_t *vfs); +bool filesystem_is_writable_by_usb(fs_user_mount_t *vfs); #endif // MICROPY_INCLUDED_SUPERVISOR_FILESYSTEM_H diff --git a/supervisor/flash.h b/supervisor/flash.h index ae6a44fd01..edf43f4b10 100644 --- a/supervisor/flash.h +++ b/supervisor/flash.h @@ -37,7 +37,6 @@ #include "supervisor/internal_flash.h" #endif -void supervisor_flash_set_usb_writable(bool usb_writable); void supervisor_flash_init(void); uint32_t supervisor_flash_get_block_size(void); uint32_t supervisor_flash_get_block_count(void); diff --git a/supervisor/shared/filesystem.c b/supervisor/shared/filesystem.c index 3382d28be8..0ef978ef21 100644 --- a/supervisor/shared/filesystem.c +++ b/supervisor/shared/filesystem.c @@ -24,6 +24,8 @@ * THE SOFTWARE. */ +#include "supervisor/filesystem.h" + #include "extmod/vfs_fat.h" #include "lib/oofatfs/ff.h" #include "lib/oofatfs/diskio.h" @@ -92,16 +94,42 @@ void filesystem_flush(void) { supervisor_flash_flush(); } -void filesystem_writable_by_python(bool writable) { +void filesystem_set_internal_writable_by_usb(bool writable) { fs_user_mount_t *vfs = &_internal_vfs; - if (!writable) { + filesystem_set_writable_by_usb(vfs, writable); +} + +void filesystem_set_writable_by_usb(fs_user_mount_t *vfs, bool usb_writable) { + if (usb_writable) { vfs->flags |= FSUSER_USB_WRITABLE; } else { vfs->flags &= ~FSUSER_USB_WRITABLE; } } +bool filesystem_is_writable_by_python(fs_user_mount_t *vfs) { + return (vfs->flags & FSUSER_CONCURRENT_WRITE_PROTECTED) == 0 || + (vfs->flags & FSUSER_USB_WRITABLE) == 0; +} + +bool filesystem_is_writable_by_usb(fs_user_mount_t *vfs) { + return (vfs->flags & FSUSER_CONCURRENT_WRITE_PROTECTED) == 0 || + (vfs->flags & FSUSER_USB_WRITABLE) != 0; +} + +void filesystem_set_internal_concurrent_write_protection(bool concurrent_write_protection) { + filesystem_set_concurrent_write_protection(&_internal_vfs, concurrent_write_protection); +} + +void filesystem_set_concurrent_write_protection(fs_user_mount_t *vfs, bool concurrent_write_protection) { + if (concurrent_write_protection) { + vfs->flags |= FSUSER_CONCURRENT_WRITE_PROTECTED; + } else { + vfs->flags &= ~FSUSER_CONCURRENT_WRITE_PROTECTED; + } +} + bool filesystem_present(void) { return true; } diff --git a/supervisor/shared/flash.c b/supervisor/shared/flash.c index 8be21e5d06..6b1f24b4bc 100644 --- a/supervisor/shared/flash.c +++ b/supervisor/shared/flash.c @@ -33,26 +33,6 @@ #define PART1_START_BLOCK (0x1) -void supervisor_flash_set_usb_writable(bool usb_writable) { - mp_vfs_mount_t* current_mount = MP_STATE_VM(vfs_mount_table); - for (uint8_t i = 0; current_mount != NULL; i++) { - if (i == VFS_INDEX) { - break; - } - current_mount = current_mount->next; - } - if (current_mount == NULL) { - return; - } - fs_user_mount_t *vfs = (fs_user_mount_t *) current_mount->obj; - - if (usb_writable) { - vfs->flags |= FSUSER_USB_WRITABLE; - } else { - vfs->flags &= ~FSUSER_USB_WRITABLE; - } -} - // there is a singleton Flash object const mp_obj_type_t supervisor_flash_type; STATIC const mp_obj_base_t supervisor_flash_obj = {&supervisor_flash_type}; diff --git a/supervisor/shared/usb/usb_msc_flash.c b/supervisor/shared/usb/usb_msc_flash.c index 658aab0d87..f6f0fa9c34 100644 --- a/supervisor/shared/usb/usb_msc_flash.c +++ b/supervisor/shared/usb/usb_msc_flash.c @@ -34,6 +34,7 @@ #include "lib/oofatfs/ff.h" #include "py/mpstate.h" +#include "supervisor/filesystem.h" #include "supervisor/shared/autoreload.h" #define MSC_FLASH_BLOCK_SIZE 512 @@ -148,8 +149,7 @@ bool tud_msc_is_writable_cb(uint8_t lun) { if (vfs == NULL) { return false; } - if (vfs->writeblocks[0] == MP_OBJ_NULL || - (vfs->flags & FSUSER_USB_WRITABLE) == 0) { + if (vfs->writeblocks[0] == MP_OBJ_NULL || !filesystem_is_writable_by_usb(vfs)) { return false; } return true; From daee83c10bc65a98f5574017e4e3fbeb350ffbe8 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Thu, 21 Feb 2019 13:23:02 -0800 Subject: [PATCH 2/3] Fix mount doc --- shared-bindings/storage/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-bindings/storage/__init__.c b/shared-bindings/storage/__init__.c index d7af48df00..4db3a9feca 100644 --- a/shared-bindings/storage/__init__.c +++ b/shared-bindings/storage/__init__.c @@ -48,7 +48,7 @@ //| directly. //| -//| .. function:: mount(filesystem, mount_path, \*, readonly=False, disable_concurrent_write_protection=False) +//| .. function:: mount(filesystem, mount_path, \*, readonly=False) //| //| Mounts the given filesystem object at the given path. //| From ed1ace09e971b24bc9ff88defe2e128064780824 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Thu, 21 Feb 2019 13:23:28 -0800 Subject: [PATCH 3/3] Fix unix build by using filesystem stub --- ports/unix/Makefile | 1 + supervisor/stub/filesystem.c | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ports/unix/Makefile b/ports/unix/Makefile index 99b65ec1bc..05b1fcaf8e 100644 --- a/ports/unix/Makefile +++ b/ports/unix/Makefile @@ -149,6 +149,7 @@ SRC_C = \ alloc.c \ coverage.c \ fatfs_port.c \ + supervisor/stub/filesystem.c \ supervisor/stub/serial.c \ supervisor/stub/stack.c \ supervisor/shared/translate.c \ diff --git a/supervisor/stub/filesystem.c b/supervisor/stub/filesystem.c index 12fe9fb109..1c4a3e12dd 100644 --- a/supervisor/stub/filesystem.c +++ b/supervisor/stub/filesystem.c @@ -26,13 +26,17 @@ #include "supervisor/filesystem.h" -void filesystem_init(void) { +void filesystem_init(bool create_allowed, bool force_create) { + (void) create_allowed; + (void) force_create; } void filesystem_flush(void) { } -void filesystem_writable_by_python(bool writable) { +bool filesystem_is_writable_by_python(fs_user_mount_t *vfs) { + (void) vfs; + return true; } bool filesystem_present(void) {