Obtain the lock on the file system object prior to calling disk_status().

[alexdevemb]

The original source code this repository is based upon calls the lock function (the equivalent of ENTER_FF macro) prior to calling the disk_status function. The inverted sequencing in the current implementation may result in unsafe access to the driver layer when used in a multitasking environment.

This inverted sequencing was observed to cause an intermittent error at the driver layer, as the driver implementation in question assumed that it was being called from a task-safe context.

Rather than implementing an additional mutex at the driver layer, it would be preferable to address the issue with the existing sync object by restoring the previous sequencing.

Suggested Fix:

static
FRESULT validate (  /* Returns FR_OK or FR_INVALID_OBJECT */
    _FDID* obj,     /* Pointer to the _OBJ, the 1st member in the FIL/DIR object, to check validity */
    FATFS** fs      /* Pointer to pointer to the owner file system object to return */
)
{
    /* Pre-validate the obj input, prior to potentially using it for the ENTER_FF or disk_status operations */
    if (!obj || !obj->fs || !obj->fs->fs_type || obj->fs->id != obj->id) {
        *fs = 0;
        return FR_INVALID_OBJECT;
    }

    /* Call the ENTER_FF macro prior to performing any possible disk operations */
    ENTER_FF(obj->fs);

    /* If the disk is not initialized, return early by calling the LEAVE_FF macro */
    if (disk_status(obj->fs->drv) & STA_NOINIT) {
        *fs = 0;
        LEAVE_FF(obj->fs, FR_INVALID_OBJECT);
    }

    *fs = obj->fs;

    return FR_OK;
}

Note, from the original source which shows the preferable sequencing (lock called prior to the disk_status call):

static
FRESULT validate (  /* Returns FR_OK or FR_INVALID_OBJECT */
    _FDID* obj,     /* Pointer to the _OBJ, the 1st member in the FIL/DIR object, to check validity */
    FATFS** fs      /* Pointer to pointer to the owner file system object to return */
)
{
    FRESULT res = FR_INVALID_OBJECT;

    if (obj && obj->fs && obj->fs->fs_type && obj->id == obj->fs->id) { /* Test if the object is valid */
#if _FS_REENTRANT
        if (lock_fs(obj->fs)) { /* Obtain the filesystem object */
            if (!(disk_status(obj->fs->drv) & STA_NOINIT)) { /* Test if the phsical drive is kept initialized */
                res = FR_OK;
            } else {
                unlock_fs(obj->fs, FR_OK);
            }
        } else {
            res = FR_TIMEOUT;
        }
#else
        if (!(disk_status(obj->fs->drv) & STA_NOINIT)) { /* Test if the phsical drive is kept initialized */
            res = FR_OK;
        }
#endif
    }
    *fs = (res == FR_OK) ? obj->fs : 0; /* Corresponding filesystem object */
    return res;
}

[fpistm]

Hi @AlexDoucette As stated in the README.md, this library is based on the https://github.com/STMicroelectronics/stm32_mw_fatfs we only adapt it as Arduino library, so far no change done here. So issue should be raised in the repo then it will be inherited here. Anyway, it seems the current code in this repo is not aligned :roll_eyes: and don't know why. I have to check that.

[fpistm]

Hi @AlexDoucette I've updated the library to be inline with the FatFs modified by ST. Could you check if it is ok for you.

[fpistm]

Anyway, as stated to get your proposal fix you should submit the issue to https://github.com/STMicroelectronics/stm32_mw_fatfs I close this one as it will be inherited and validate by official stm32_mw_fatfs maintainer.