[U-Boot] [PATCH 3/4 v2] UBIFS: Change ubifsload to not read beyond the requested size

Until now ubifsload pads the destination with 0 up to a multiple of UBIFS_BLOCK_SIZE (4KiB) while reading a file to memory. This patch changes this behaviour to only read to the requested length. This is either the file length or the length/size provided as parameter to the ubifsload command.
Signed-off-by: Stefan Roese sr@denx.de --- v2: - Added error checking to malloc and read_block as suggested by Wolfgang - Changed comment issues as suggested by Ben
fs/ubifs/ubifs.c | 71 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 58 insertions(+), 13 deletions(-)
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 1cc31a9..d16d2b0 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -3,7 +3,7 @@ * * Copyright (C) 2006-2008 Nokia Corporation. * - * (C) Copyright 2008-2009 + * (C) Copyright 2008-2010 * Stefan Roese, DENX Software Engineering, sr@denx.de. * * This program is free software; you can redistribute it and/or modify it @@ -567,7 +567,8 @@ dump: return -EINVAL; }
-static int do_readpage(struct ubifs_info *c, struct inode *inode, struct page *page) +static int do_readpage(struct ubifs_info *c, struct inode *inode, + struct page *page, int last_block_size) { void *addr; int err = 0, i; @@ -601,17 +602,54 @@ static int do_readpage(struct ubifs_info *c, struct inode *inode, struct page *p err = -ENOENT; memset(addr, 0, UBIFS_BLOCK_SIZE); } else { - ret = read_block(inode, addr, block, dn); - if (ret) { - err = ret; - if (err != -ENOENT) + /* + * Reading last block? Make sure to not write beyond + * the requested size in the destination buffer. + */ + if (((block + 1) == beyond) || last_block_size) { + void *buff; + int dlen; + + /* + * We need to buffer the data locally for the + * last block. This is to not pad the + * destination area to a multiple of + * UBIFS_BLOCK_SIZE. + */ + buff = malloc(UBIFS_BLOCK_SIZE); + if (!buff) { + printf("%s: Error, malloc fails!\n", + __func__); + err = -ENOMEM; break; - } else if (block + 1 == beyond) { - int dlen = le32_to_cpu(dn->size); - int ilen = i_size & (UBIFS_BLOCK_SIZE - 1); - - if (ilen && ilen < dlen) - memset(addr + ilen, 0, dlen - ilen); + } + + /* Read block-size into temp buffer */ + ret = read_block(inode, buff, block, dn); + if (ret) { + err = ret; + if (err != -ENOENT) { + free(buff); + break; + } + } + + if (last_block_size) + dlen = last_block_size; + else + dlen = le32_to_cpu(dn->size); + + /* Now copy required size back to dest */ + memcpy(addr, buff, dlen); + + free(buff); + } else { + ret = read_block(inode, addr, block, dn); + if (ret) { + err = ret; + if (err != -ENOENT) + break; + } } } if (++i >= UBIFS_BLOCKS_PER_PAGE) @@ -649,6 +687,7 @@ int ubifs_load(char *filename, u32 addr, u32 size) int err = 0; int i; int count; + int last_block_size = 0;
c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READONLY); /* ubifs_findfile will resolve symlinks, so we know that we get @@ -684,7 +723,13 @@ int ubifs_load(char *filename, u32 addr, u32 size) page.index = 0; page.inode = inode; for (i = 0; i < count; i++) { - err = do_readpage(c, inode, &page); + /* + * Make sure to not read beyond the requested size + */ + if (((i + 1) == count) && (size < inode->i_size)) + last_block_size = size - (i * PAGE_SIZE); + + err = do_readpage(c, inode, &page, last_block_size); if (err) break;

Dear Stefan Roese,
In message 1288628880-6910-1-git-send-email-sr@denx.de you wrote:
/* Read block-size into temp buffer */
ret = read_block(inode, buff, block, dn);
if (ret) {
err = ret;
if (err != -ENOENT) {
free(buff);
break;
}
}
...
ret = read_block(inode, addr, block, dn);
if (ret) {
err = ret;
if (err != -ENOENT)
break;
}
I still wonder what's the logic behind this code. When will read_block() return -ENOENT (aka "No such file or directory") ? What are the other possible error conditions, and why would it make sense to continue reading after these other errors?
Best regards,
Wolfgang Denk

Hi Wolfgang,
sorry for the late reply. I just "stumbled" again over this mail.
On Monday 01 November 2010 20:05:00 Wolfgang Denk wrote:
I still wonder what's the logic behind this code. When will read_block() return -ENOENT (aka "No such file or directory") ? What are the other possible error conditions, and why would it make sense to continue reading after these other errors?
As it seems, ENOENT is used to mark "a hole" in the file system. Meaning space that will be filled with zeros but does not occupy space (other than in the index). So we should keep the existing logic intact.
Cheers, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de

On Monday 01 November 2010 17:28:00 Stefan Roese wrote:
Until now ubifsload pads the destination with 0 up to a multiple of UBIFS_BLOCK_SIZE (4KiB) while reading a file to memory. This patch changes this behaviour to only read to the requested length. This is either the file length or the length/size provided as parameter to the ubifsload command.
Applied to u-boot-ubi/master. Thanks.
Cheers, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
participants (2)
-
Stefan Roese
-
Wolfgang Denk