search

tuxera / reliance-edge

Transactional power-failsafe filesystem for microcontrollers
https://www.tuxera.com/company/open-source
GNU General Public License v2.0
109 stars 32 forks source link

FreeRTOS F_DRIVER bug in block driver read/write #1

Closed aamcampbell closed 9 years ago

aamcampbell commented 9 years ago

The FreeRTOS F_DRIVER block device read and write functions use the wrong variable(s) for the sector index.

From os/freertos/services/osbdev.c, line 478 (DiskRead()):

    for(ulSectorIdx = 0U; ulSectorIdx < ulSectorCount; ulSectorIdx++)
    {
        iErr = pDriver->readsector(pDriver, &pbBuffer[ulSectorIdx * ulSectorSize],
                                   CAST_ULONG(ullSectorStart + ulSectorCount));
    /*...*/
    }

Line 534 contains similar logic for the DiskWrite() function.

The F_DRIVER readsector and writesector methods can only handle one sector at a time, so they are called from within a for loop so that multiple sectors can be transferred by the RedOsBDevRead and Write methods. The loop iterator is ulSectorIdx, which is correctly used to find the write place in the given buffer. However, the loop iterator is ignored when determining the sector number pass to readsector and writesector. Instead, (ullSectorStart + ulSectorCount) is passed as the sector number.

The values of ullSectorStart and ulSectorCount are not modified within the loop. For multi-sector transfers, this means that the same sector will be read or written repetitively instead of the expected number of sectors being transferred to or from the disk. Furthermore, the sector being accessed is always one beyond the end of the requested transfer, which affects single-sector transfers as well. For example, on single-sector transfers (ulSectorCount = 1), the desired sector is at ullSectorStart, but ullSectorStart + 1 is used as the sector number.

The FreeRTOS BDEV_F_DRIVER implementations of DiskRead and DiskWrite (os/freertos/services/osbdev.c) are affected by this bug. Other block device service implementations are not affected.

This issue report is for documentation purposes only; the bug has already been fixed.

aamcampbell commented 9 years ago

The correct value to pass to readsector() and writesector() is (ullSectorStart + ulSectorIdx). This change has been made to both the DiskRead() and DiskWrite() methods. See commit 9104a7ed (Trunk Build 665).