Grab FS lock to test writability

Checking is_writable_from_python always follows USB policy even if
it is unplugged. Fixes #8986

Also does bare minimum for #8961.

Author: tannewt

extmod/vfs_fat.c

@@ -99,7 +99,7 @@ STATIC mp_obj_t fat_vfs_make_new(const mp_obj_type_t *type, size_t n_args, size_
 }
 
 STATIC void verify_fs_writable(fs_user_mount_t *vfs) {
-    if (!filesystem_is_writable_by_python(vfs)) {
+    if (!filesystem_lock(vfs)) {
         mp_raise_OSError(MP_EROFS);
     }
 }
@@ -218,7 +218,6 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(fat_vfs_ilistdir_obj, 1, 2, fat_vfs_i
 
 STATIC mp_obj_t fat_vfs_remove_internal(mp_obj_t vfs_in, mp_obj_t path_in, mp_int_t attr) {
     mp_obj_fat_vfs_t *self = MP_OBJ_TO_PTR(vfs_in);
-    verify_fs_writable(self);
     const char *path = mp_obj_str_get_str(path_in);
 
     FILINFO fno;
@@ -230,7 +229,9 @@ STATIC mp_obj_t fat_vfs_remove_internal(mp_obj_t vfs_in, mp_obj_t path_in, mp_in
 
     // check if path is a file or directory
     if ((fno.fattrib & AM_DIR) == attr) {
+        verify_fs_writable(self);
         res = f_unlink(&self->fatfs, path);
+        filesystem_unlock(self);
 
         if (res != FR_OK) {
             mp_raise_OSError_fresult(res);
@@ -253,17 +254,18 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_2(fat_vfs_rmdir_obj, fat_vfs_rmdir);
 
 STATIC mp_obj_t fat_vfs_rename(mp_obj_t vfs_in, mp_obj_t path_in, mp_obj_t path_out) {
     mp_obj_fat_vfs_t *self = MP_OBJ_TO_PTR(vfs_in);
-    verify_fs_writable(self);
     const char *old_path = mp_obj_str_get_str(path_in);
     const char *new_path = mp_obj_str_get_str(path_out);
 
+    verify_fs_writable(self);
     FRESULT res = f_rename(&self->fatfs, old_path, new_path);
     if (res == FR_EXIST) {
         // if new_path exists then try removing it (but only if it's a file)
         fat_vfs_remove_internal(vfs_in, path_out, 0); // 0 == file attribute
         // try to rename again
         res = f_rename(&self->fatfs, old_path, new_path);
     }
+    filesystem_unlock(self);
     if (res == FR_OK) {
         return mp_const_none;
     } else {
@@ -275,9 +277,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_3(fat_vfs_rename_obj, fat_vfs_rename);
 
 STATIC mp_obj_t fat_vfs_mkdir(mp_obj_t vfs_in, mp_obj_t path_o) {
     mp_obj_fat_vfs_t *self = MP_OBJ_TO_PTR(vfs_in);
-    verify_fs_writable(self);
     const char *path = mp_obj_str_get_str(path_o);
+    verify_fs_writable(self);
     FRESULT res = f_mkdir(&self->fatfs, path);
+    filesystem_unlock(self);
     if (res == FR_OK) {
         return mp_const_none;
     } else {
@@ -463,7 +466,11 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_3(fat_vfs_utime_obj, vfs_fat_utime);
 
 STATIC mp_obj_t vfs_fat_getreadonly(mp_obj_t self_in) {
     fs_user_mount_t *self = MP_OBJ_TO_PTR(self_in);
-    return mp_obj_new_bool(!filesystem_is_writable_by_python(self));
+    bool writable = filesystem_lock(self);
+    if (writable) {
+        filesystem_unlock(self);
+    }
+    return mp_obj_new_bool(!writable);
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(fat_vfs_getreadonly_obj, vfs_fat_getreadonly);
 STATIC const mp_obj_property_t fat_vfs_readonly_obj = {
@@ -487,9 +494,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(fat_vfs_getlabel_obj, vfs_fat_getlabel);
 
 STATIC mp_obj_t vfs_fat_setlabel(mp_obj_t self_in, mp_obj_t label_in) {
     fs_user_mount_t *self = MP_OBJ_TO_PTR(self_in);
-    verify_fs_writable(self);
     const char *label_str = mp_obj_str_get_str(label_in);
+    verify_fs_writable(self);
     FRESULT res = f_setlabel(&self->fatfs, label_str);
+    filesystem_unlock(self);
     if (res != FR_OK) {
         if (res == FR_WRITE_PROTECTED) {
             mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("Read-only filesystem"));

extmod/vfs_fat.h

@@ -50,6 +50,8 @@ MP_DECLARE_CONST_FUN_OBJ_3(fat_vfs_open_obj);
 typedef struct _pyb_file_obj_t {
     mp_obj_base_t base;
     FIL fp;
+    // CIRCUITPY-CHANGE: We need to unlock the fs on file close.
+    fs_user_mount_t *fs_mount;
 } pyb_file_obj_t;
 
 #endif  // MICROPY_INCLUDED_EXTMOD_VFS_FAT_H

extmod/vfs_fat_file.c

@@ -127,6 +127,9 @@ STATIC mp_uint_t file_obj_ioctl(mp_obj_t o_in, mp_uint_t request, uintptr_t arg,
     } else if (request == MP_STREAM_CLOSE) {
         // if fs==NULL then the file is closed and in that case this method is a no-op
         if (self->fp.obj.fs != NULL) {
+            if (self->fp.flag & FA_WRITE) {
+                filesystem_unlock(self->fs_mount);
+            }
             FRESULT res = f_close(&self->fp);
             if (res != FR_OK) {
                 *errcode = fresult_to_errno_table[res];
@@ -243,13 +246,14 @@ STATIC mp_obj_t fat_vfs_open(mp_obj_t self_in, mp_obj_t path_in, mp_obj_t mode_i
     }
 
     assert(self != NULL);
-    if ((mode & FA_WRITE) != 0 && !filesystem_is_writable_by_python(self)) {
+    if ((mode & FA_WRITE) != 0 && !filesystem_lock(self)) {
         mp_raise_OSError(MP_EROFS);
     }
 
 
     pyb_file_obj_t *o = m_new_obj_with_finaliser(pyb_file_obj_t);
     o->base.type = type;
+    o->fs_mount = self;
 
     const char *fname = mp_obj_str_get_str(path_in);
     FRESULT res = f_open(&self->fatfs, &o->fp, fname, mode);

ports/espressif/supervisor/internal_flash.c

@@ -151,7 +151,7 @@ mp_uint_t supervisor_flash_write_blocks(const uint8_t *src, uint32_t lba, uint32
         uint32_t block_address = lba + block;
         uint32_t sector_offset = block_address / blocks_per_sector * SECTOR_SIZE;
         uint8_t block_offset = block_address % blocks_per_sector;
-        if (_cache_lba != block_address) {
+        if (_cache_lba != sector_offset) {
             supervisor_flash_read_blocks(_cache, sector_offset / FILESYSTEM_BLOCK_SIZE, blocks_per_sector);
             _cache_lba = sector_offset;
         }

ports/raspberrypi/supervisor/internal_flash.c

@@ -127,7 +127,7 @@ mp_uint_t supervisor_flash_write_blocks(const uint8_t *src, uint32_t lba, uint32
         uint32_t sector_offset = block_address / blocks_per_sector * SECTOR_SIZE;
         uint8_t block_offset = block_address % blocks_per_sector;
 
-        if (_cache_lba != block_address) {
+        if (_cache_lba != sector_offset) {
             port_internal_flash_flush();
             memcpy(_cache,
                 (void *)(XIP_BASE + CIRCUITPY_CIRCUITPY_DRIVE_START_ADDR + sector_offset),

supervisor/filesystem.h

@@ -41,7 +41,10 @@ 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);
+
+// This controls whether USB tries to grab the underlying block device lock
+// during enumeration. If another workflow is modifying the filesystem when this
+// happens, then USB will be readonly.
 bool filesystem_is_writable_by_usb(fs_user_mount_t *vfs);
 
 fs_user_mount_t *filesystem_circuitpy(void);

supervisor/shared/filesystem.c

@@ -208,11 +208,6 @@ void filesystem_set_writable_by_usb(fs_user_mount_t *vfs, bool usb_writable) {
     }
 }
 
-bool filesystem_is_writable_by_python(fs_user_mount_t *vfs) {
-    return (vfs->blockdev.flags & MP_BLOCKDEV_FLAG_CONCURRENT_WRITE_PROTECTED) == 0 ||
-           (vfs->blockdev.flags & MP_BLOCKDEV_FLAG_USB_WRITABLE) == 0;
-}
-
 bool filesystem_is_writable_by_usb(fs_user_mount_t *vfs) {
     return (vfs->blockdev.flags & MP_BLOCKDEV_FLAG_CONCURRENT_WRITE_PROTECTED) == 0 ||
            (vfs->blockdev.flags & MP_BLOCKDEV_FLAG_USB_WRITABLE) != 0;

supervisor/shared/web_workflow/web_workflow.c

@@ -698,7 +698,11 @@ static void _reply_directory_json(socketpool_socket_obj_t *socket, _request *req
     uint32_t total_clusters = fatfs->n_fatent - 2;
 
     const char *writable = "false";
-    if (filesystem_is_writable_by_python(fs_mount)) {
+    // Test to see if we can grab the write lock. USB will grab the underlying
+    // blockdev lock once it says it is writable. Unlock immediately since we
+    // aren't actually writing.
+    if (filesystem_lock(fs_mount)) {
+        filesystem_unlock(fs_mount);
         writable = "true";
     }
     mp_printf(&_socket_print,
@@ -920,7 +924,8 @@ static void _reply_with_diskinfo_json(socketpool_socket_obj_t *socket, _request
         size_t total_size = fatfs->n_fatent - 2;
 
         const char *writable = "false";
-        if (filesystem_is_writable_by_python(fs)) {
+        if (filesystem_lock(fs)) {
+            filesystem_unlock(fs);
             writable = "true";
         }
         mp_printf(&_socket_print,