[U-Boot] [PATCH v2 1/2] fs/fat: Fix 'CACHE: Misaligned operation at range' warnings

The 'block' field of fat_itr needs to be properly aligned for DMA and while it does have '__aligned(ARCH_DMA_MINALIGN)', the fat_itr structure itself needs to be properly aligned as well.
While at it use malloc_cache_aligned() for the other aligned allocations in the file as well.
Fixes: 2460098cffacd1 ("fs/fat: Reduce stack usage") Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi --- v2: No change --- fs/fat/fat.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 3d3e17e8fa..35941c1498 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -495,7 +495,7 @@ read_bootsectandvi(boot_sector *bs, volume_info *volinfo, int *fatsize) return -1; }
- block = memalign(ARCH_DMA_MINALIGN, cur_dev->blksz); + block = malloc_cache_aligned(cur_dev->blksz); if (block == NULL) { debug("Error: allocating block\n"); return -1; @@ -599,7 +599,7 @@ static int get_fs_info(fsdata *mydata)
mydata->fatbufnum = -1; mydata->fat_dirty = 0; - mydata->fatbuf = memalign(ARCH_DMA_MINALIGN, FATBUFSIZE); + mydata->fatbuf = malloc_cache_aligned(FATBUFSIZE); if (mydata->fatbuf == NULL) { debug("Error: allocating memory\n"); return -1; @@ -1038,7 +1038,7 @@ int fat_exists(const char *filename) fat_itr *itr; int ret;
- itr = malloc(sizeof(fat_itr)); + itr = malloc_cache_aligned(sizeof(fat_itr)); ret = fat_itr_root(itr, &fsdata); if (ret) return 0; @@ -1055,7 +1055,7 @@ int fat_size(const char *filename, loff_t *size) fat_itr *itr; int ret;
- itr = malloc(sizeof(fat_itr)); + itr = malloc_cache_aligned(sizeof(fat_itr)); ret = fat_itr_root(itr, &fsdata); if (ret) return ret; @@ -1089,7 +1089,7 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer, fat_itr *itr; int ret;
- itr = malloc(sizeof(fat_itr)); + itr = malloc_cache_aligned(sizeof(fat_itr)); ret = fat_itr_root(itr, &fsdata); if (ret) return ret;

Check malloc() return values and properly unwind on errors so memory allocated for fat_itr structures get freed properly.
Also fixes a leak of fsdata.fatbuf in fat_size().
Fixes: 2460098cffacd1 ("fs/fat: Reduce stack usage") Reported-by: Coverity (CID: 167225, 167233, 167234) Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi --- v2: New patch --- fs/fat/fat.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 35941c1498..7fe78439cf 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -1039,12 +1039,15 @@ int fat_exists(const char *filename) int ret;
itr = malloc_cache_aligned(sizeof(fat_itr)); + if (!itr) + return 0; ret = fat_itr_root(itr, &fsdata); if (ret) - return 0; + goto out;
ret = fat_itr_resolve(itr, filename, TYPE_ANY); free(fsdata.fatbuf); +out: free(itr); return ret == 0; } @@ -1056,9 +1059,11 @@ int fat_size(const char *filename, loff_t *size) int ret;
itr = malloc_cache_aligned(sizeof(fat_itr)); + if (!itr) + return -ENOMEM; ret = fat_itr_root(itr, &fsdata); if (ret) - return ret; + goto out_free_itr;
ret = fat_itr_resolve(itr, filename, TYPE_FILE); if (ret) { @@ -1072,12 +1077,13 @@ int fat_size(const char *filename, loff_t *size) *size = 0; ret = 0; } - goto out; + goto out_free_both; }
*size = FAT2CPU32(itr->dent->size); +out_free_both: free(fsdata.fatbuf); -out: +out_free_itr: free(itr); return ret; } @@ -1090,19 +1096,22 @@ int file_fat_read_at(const char *filename, loff_t pos, void *buffer, int ret;
itr = malloc_cache_aligned(sizeof(fat_itr)); + if (!itr) + return -ENOMEM; ret = fat_itr_root(itr, &fsdata); if (ret) - return ret; + goto out_free_itr;
ret = fat_itr_resolve(itr, filename, TYPE_FILE); if (ret) - goto out; + goto out_free_both;
printf("reading %s\n", filename); ret = get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
-out: +out_free_both: free(fsdata.fatbuf); +out_free_itr: free(itr); return ret; } @@ -1148,17 +1157,18 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp)
ret = fat_itr_root(&dir->itr, &dir->fsdata); if (ret) - goto fail; + goto fail_free_dir;
ret = fat_itr_resolve(&dir->itr, filename, TYPE_DIR); if (ret) - goto fail; + goto fail_free_both;
*dirsp = (struct fs_dir_stream *)dir; return 0;
-fail: +fail_free_both: free(dir->fsdata.fatbuf); +fail_free_dir: free(dir); return ret; }

On Sun, Oct 01, 2017 at 02:25:22AM +0300, Tuomas Tynkkynen wrote:
Check malloc() return values and properly unwind on errors so memory allocated for fat_itr structures get freed properly.
Also fixes a leak of fsdata.fatbuf in fat_size().
Fixes: 2460098cffacd1 ("fs/fat: Reduce stack usage") Reported-by: Coverity (CID: 167225, 167233, 167234) Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
Reviewed-by: Tom Rini trini@konsulko.com

On Sun, Oct 01, 2017 at 02:25:22AM +0300, Tuomas Tynkkynen wrote:
Check malloc() return values and properly unwind on errors so memory allocated for fat_itr structures get freed properly.
Also fixes a leak of fsdata.fatbuf in fat_size().
Fixes: 2460098cffacd1 ("fs/fat: Reduce stack usage") Reported-by: Coverity (CID: 167225, 167233, 167234) Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!

On Sun, Oct 01, 2017 at 02:25:21AM +0300, Tuomas Tynkkynen wrote:
The 'block' field of fat_itr needs to be properly aligned for DMA and while it does have '__aligned(ARCH_DMA_MINALIGN)', the fat_itr structure itself needs to be properly aligned as well.
While at it use malloc_cache_aligned() for the other aligned allocations in the file as well.
Fixes: 2460098cffacd1 ("fs/fat: Reduce stack usage") Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
Reviewed-by: Tom Rini trini@konsulko.com

On Sun, Oct 01, 2017 at 02:25:21AM +0300, Tuomas Tynkkynen wrote:
The 'block' field of fat_itr needs to be properly aligned for DMA and while it does have '__aligned(ARCH_DMA_MINALIGN)', the fat_itr structure itself needs to be properly aligned as well.
While at it use malloc_cache_aligned() for the other aligned allocations in the file as well.
Fixes: 2460098cffacd1 ("fs/fat: Reduce stack usage") Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!
participants (2)
-
Tom Rini
-
Tuomas Tynkkynen