New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Sector/Size overflow from uint32_t #5829

Merged
merged 1 commit into from Jan 15, 2018

Conversation

Projects
None yet
5 participants
@deepikabhavnani
Contributor

deepikabhavnani commented Jan 11, 2018

FATFilesystem declares sector count and size as uint32_t and block device class arguments are addr and size which is uint64_t. While passing arguments to program/read/write API's of block device, multiplication of uint32_t*uint32_t was not type-casted to uint64_t which resulted in truncation.

Example of FAT corruption:
If sector 0x800000 is accessed with block size 0x200, addr to be passed 0x1 0000 0000 (0x800000*0x200), but actual address passed was 0x0000 0000 which resulted in over-writting the root directory, and hence corrupted filesystem.

@deepikabhavnani deepikabhavnani requested a review from geky Jan 11, 2018

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Jan 11, 2018

This should resolve issue 3 in #5780

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:fat_issue_5780_3 branch from b965e7e to eafa8a7 Jan 11, 2018

@geky

geky approved these changes Jan 12, 2018

Looks good to me 👍
Thanks for hunting down this fix

@@ -196,20 +196,24 @@ DRESULT disk_read(BYTE pdrv, BYTE *buff, DWORD sector, UINT count)
{
debug_if(FFS_DBG, "disk_read(sector %d, count %d) on pdrv [%d]\n", sector, count, pdrv);
DWORD ssize = disk_get_sector_size(pdrv);
int err = _ffs[pdrv]->read(buff, sector*ssize, count*ssize);
bd_addr_t addr = (bd_addr_t)sector * ssize;
bd_size_t size = (bd_size_t)count*ssize;

This comment has been minimized.

@geky

geky Jan 12, 2018

Member

nit: consistent style for these two lines.

return err ? RES_PARERR : RES_OK;
}
DRESULT disk_write(BYTE pdrv, const BYTE *buff, DWORD sector, UINT count)
{
debug_if(FFS_DBG, "disk_write(sector %d, count %d) on pdrv [%d]\n", sector, count, pdrv);
DWORD ssize = disk_get_sector_size(pdrv);
int err = _ffs[pdrv]->erase(sector*ssize, count*ssize);
bd_addr_t addr = (bd_addr_t)sector * ssize;
bd_size_t size = (bd_size_t)count*ssize;

This comment has been minimized.

@geky

geky Jan 12, 2018

Member

nit: consistent style for these two lines

Fix: Sector/Size overflow from uint32_t
FATFilesystem declares sector count and size as uint32_t and block
device class arguments are addr and size which is uint64_t
While passing arguments to program/read/write API's of block device,
multiplication of uint32_t*uint32_t was not typecasted properly to
uint64_t which resulted in MSB truncation.

Eg. If block 0x800000 is accessed with block size 0x200, addr to be
passed (0x800000*0x200)0x100000000, but actual address passed was 0x0
which resulted in over-writting the root directory, and hence corrupted
filesystem

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:fat_issue_5780_3 branch from eafa8a7 to c86d757 Jan 12, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 12, 2018

/morph build

@cmonr cmonr added needs: CI and removed needs: review labels Jan 12, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Jan 12, 2018

Build : SUCCESS

Build number : 859
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5829/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 12, 2018

/morph uvisor-test

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 15, 2018

Travis restarted due to failing update command, once green, it shall be ready

@0xc0170 0xc0170 merged commit 8c78649 into ARMmbed:master Jan 15, 2018

19 checks passed

ARM mbed CI Verification build successful.
Details
AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Local littlefs testing has passed
Details
travis-ci/mbed2-ATMEL Local mbed2-ATMEL testing has passed
Details
travis-ci/mbed2-MAXIM Local mbed2-MAXIM testing has passed
Details
travis-ci/mbed2-NORDIC Local mbed2-NORDIC testing has passed
Details
travis-ci/mbed2-NUVOTON Local mbed2-NUVOTON testing has passed
Details
travis-ci/mbed2-NXP Local mbed2-NXP testing has passed
Details
travis-ci/mbed2-RENESAS Local mbed2-RENESAS testing has passed
Details
travis-ci/mbed2-SILICON_LABS Local mbed2-SILICON_LABS testing has passed
Details
travis-ci/mbed2-STM Local mbed2-STM testing has passed
Details
travis-ci/tools Local tools testing has passed
Details

@deepikabhavnani deepikabhavnani deleted the deepikabhavnani:fat_issue_5780_3 branch Jan 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment