We make these two functions take a size_t pointer to how much space was used on NAND to read or write the buffer (when reads/writes happen) so that bad blocks can be accounted for. We also make them take an loff_t limit on how much data can be read or written. This means that we can now catch the case of when writing to a partition would exceed the partition size due to bad blocks. To do this we also need to make check_skip_len count not just complete blocks used but partial ones as well. All callers of nand_(read|write)_skip_bad are adjusted to call these with the most sensible limits available.
The changes were started by Pantelis and finished by Tom.
Signed-off-by: Pantelis Antoniou panto@antoniou-consulting.com Signed-off-by: Tom Rini trini@ti.com --- Changes in v6: None Changes in v5: None Changes in v4: - Further reword nand_util.c comments, from Scott - In nand_write_skip_bad make sure *actual is 0 for YAFFS2 errors too, reminded by Scott. - In cmd_nand.c don't drop the size is less than maxsize check in arg_off_size as other nand functions need this still (Scott).
Changes in v3: - Reworked skip_check_len changes to just add accounting for *used to the logic. - Allow for actual to be NULL in nand_(read|write)_skip_bad, only DFU calls this with a non-NULL parameter. Make sure the comments for both functions explain the parameters and their behavior. - Other style changes requested by Scott. - As nand_(write|read)_skip_bad passes back just a used length now.
Changes in v2: - NAND skip_check_len changes reworked to allow nand_(read|write)_skip_bad to return this information to the caller.
common/cmd_nand.c | 53 ++++++++++++++++++-------------- common/env_nand.c | 3 +- drivers/mtd/nand/nand_util.c | 68 +++++++++++++++++++++++++++++++++++++----- include/nand.h | 4 +-- 4 files changed, 95 insertions(+), 33 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c index 32348f3..110c78c 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -137,7 +137,8 @@ static inline int str2long(const char *p, ulong *num) return *p != '\0' && *endptr == '\0'; }
-static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size) +static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size, + loff_t *maxsize) { #ifdef CONFIG_CMD_MTDPARTS struct mtd_device *dev; @@ -160,6 +161,7 @@ static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size)
*off = part->offset; *size = part->size; + *maxsize = part->size; *idx = dev->id->num;
ret = set_dev(*idx); @@ -173,10 +175,11 @@ static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size) #endif }
-static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *maxsize) +static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size, + loff_t *maxsize) { if (!str2off(arg, off)) - return get_part(arg, idx, off, maxsize); + return get_part(arg, idx, off, size, maxsize);
if (*off >= nand_info[*idx].size) { puts("Offset exceeds device limit\n"); @@ -184,36 +187,35 @@ static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *maxsize) }
*maxsize = nand_info[*idx].size - *off; + *size = *maxsize; return 0; }
static int arg_off_size(int argc, char *const argv[], int *idx, - loff_t *off, loff_t *size) + loff_t *off, loff_t *size, loff_t *maxsize) { int ret; - loff_t maxsize = 0;
if (argc == 0) { *off = 0; *size = nand_info[*idx].size; + *maxsize = *size; goto print; }
- ret = arg_off(argv[0], idx, off, &maxsize); + ret = arg_off(argv[0], idx, off, size, maxsize); if (ret) return ret;
- if (argc == 1) { - *size = maxsize; + if (argc == 1) goto print; - }
if (!str2off(argv[1], size)) { printf("'%s' is not a number\n", argv[1]); return -1; }
- if (*size > maxsize) { + if (*size > *maxsize) { puts("Size exceeds partition or device limit\n"); return -1; } @@ -307,7 +309,8 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[]) if (argc < 3) goto usage;
- if (arg_off(argv[2], &idx, &addr, &maxsize)) { + /* We don't care about size, or maxsize. */ + if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) { puts("Offset or partition name expected\n"); return 1; } @@ -426,7 +429,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { int i, ret = 0; ulong addr; - loff_t off, size; + loff_t off, size, maxsize; char *cmd, *s; nand_info_t *nand; #ifdef CONFIG_SYS_NAND_QUIET @@ -551,7 +554,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
printf("\nNAND %s: ", cmd); /* skip first two or three arguments, look for offset and size */ - if (arg_off_size(argc - o, argv + o, &dev, &off, &size) != 0) + if (arg_off_size(argc - o, argv + o, &dev, &off, &size, + &maxsize) != 0) return 1;
nand = &nand_info[dev]; @@ -619,7 +623,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (s && !strcmp(s, ".raw")) { raw = 1;
- if (arg_off(argv[3], &dev, &off, &size)) + if (arg_off(argv[3], &dev, &off, &size, &maxsize)) return 1;
if (argc > 4 && !str2long(argv[4], &pagecount)) { @@ -635,7 +639,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) rwsize = pagecount * (nand->writesize + nand->oobsize); } else { if (arg_off_size(argc - 3, argv + 3, &dev, - &off, &size) != 0) + &off, &size, &maxsize) != 0) return 1;
rwsize = size; @@ -645,9 +649,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) !strcmp(s, ".e") || !strcmp(s, ".i")) { if (read) ret = nand_read_skip_bad(nand, off, &rwsize, + NULL, maxsize, (u_char *)addr); else ret = nand_write_skip_bad(nand, off, &rwsize, + NULL, maxsize, (u_char *)addr, 0); #ifdef CONFIG_CMD_NAND_TRIMFFS } else if (!strcmp(s, ".trimffs")) { @@ -655,8 +661,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) printf("Unknown nand command suffix '%s'\n", s); return 1; } - ret = nand_write_skip_bad(nand, off, &rwsize, - (u_char *)addr, + ret = nand_write_skip_bad(nand, off, &rwsize, NULL, + maxsize, (u_char *)addr, WITH_DROP_FFS); #endif #ifdef CONFIG_CMD_NAND_YAFFS @@ -665,8 +671,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) printf("Unknown nand command suffix '%s'.\n", s); return 1; } - ret = nand_write_skip_bad(nand, off, &rwsize, - (u_char *)addr, + ret = nand_write_skip_bad(nand, off, &rwsize, NULL, + maxsize, (u_char *)addr, WITH_INLINE_OOB); #endif } else if (!strcmp(s, ".oob")) { @@ -775,7 +781,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (s && !strcmp(s, ".allexcept")) allexcept = 1;
- if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size) < 0) + if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size, + &maxsize) < 0) return 1;
if (!nand_unlock(&nand_info[dev], off, size, allexcept)) { @@ -873,7 +880,8 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand, printf("\nLoading from %s, offset 0x%lx\n", nand->name, offset);
cnt = nand->writesize; - r = nand_read_skip_bad(nand, offset, &cnt, (u_char *) addr); + r = nand_read_skip_bad(nand, offset, &cnt, NULL, nand->size, + (u_char *)addr); if (r) { puts("** Read error\n"); bootstage_error(BOOTSTAGE_ID_NAND_HDR_READ); @@ -905,7 +913,8 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand, } bootstage_mark(BOOTSTAGE_ID_NAND_TYPE);
- r = nand_read_skip_bad(nand, offset, &cnt, (u_char *) addr); + r = nand_read_skip_bad(nand, offset, &cnt, NULL, nand->size, + (u_char *)addr); if (r) { puts("** Read error\n"); bootstage_error(BOOTSTAGE_ID_NAND_READ); diff --git a/common/env_nand.c b/common/env_nand.c index 5b69889..b745822 100644 --- a/common/env_nand.c +++ b/common/env_nand.c @@ -281,7 +281,8 @@ int readenv(size_t offset, u_char *buf) } else { char_ptr = &buf[amount_loaded]; if (nand_read_skip_bad(&nand_info[0], offset, - &len, char_ptr)) + &len, NULL, + nand_info[0].size, char_ptr)) return 1;
offset += blocksize; diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index ff2d348..4727f9c 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -416,11 +416,13 @@ int nand_unlock(struct mtd_info *mtd, loff_t start, size_t length, * @param nand NAND device * @param offset offset in flash * @param length image length + * @param used length of flash needed for the requested length * @return 0 if the image fits and there are no bad blocks * 1 if the image fits, but there are bad blocks * -1 if the image does not fit */ -static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length) +static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length, + size_t *used) { size_t len_excl_bad = 0; int ret = 0; @@ -442,8 +444,13 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length) ret = 1;
offset += block_len; + *used += block_len; }
+ /* If the length is not a multiple of block_len, adjust. */ + if (len_excl_bad > length) + *used -= (len_excl_bad - length); + return ret; }
@@ -476,23 +483,36 @@ static size_t drop_ffs(const nand_info_t *nand, const u_char *buf, * Write image to NAND flash. * Blocks that are marked bad are skipped and the is written to the next * block instead as long as the image is short enough to fit even after - * skipping the bad blocks. + * skipping the bad blocks. Due to bad blocks we may not be able to + * perform the requested write. In the case where the write would + * extend beyond the end of the NAND device, both length and actual (if + * not NULL) are set to 0. In the case where the write would extend + * beyond the limit we are passed, length is set to 0 and actual is set + * to the required length. * * @param nand NAND device * @param offset offset in flash * @param length buffer length + * @param actual set to size required to write length worth of + * buffer or 0 on error, if not NULL + * @param lim maximum size that actual may be in order to not + * exceed the buffer * @param buffer buffer to read from * @param flags flags modifying the behaviour of the write to NAND * @return 0 in case of success */ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, - u_char *buffer, int flags) + size_t *actual, loff_t lim, u_char *buffer, int flags) { int rval = 0, blocksize; size_t left_to_write = *length; + size_t used_for_write = 0; u_char *p_buffer = buffer; int need_skip;
+ if (actual) + *actual = 0; + #ifdef CONFIG_CMD_NAND_YAFFS if (flags & WITH_YAFFS_OOB) { if (flags & ~WITH_YAFFS_OOB) @@ -529,13 +549,23 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, return -EINVAL; }
- need_skip = check_skip_len(nand, offset, *length); + need_skip = check_skip_len(nand, offset, *length, &used_for_write); + + if (actual) + *actual = used_for_write; + if (need_skip < 0) { printf("Attempt to write outside the flash area\n"); *length = 0; return -EINVAL; }
+ if (used_for_write > lim) { + puts("Size of write exceeds partition or device limit\n"); + *length = 0; + return -EFBIG; + } + if (!need_skip && !(flags & WITH_DROP_FFS)) { rval = nand_write(nand, offset, length, buffer); if (rval == 0) @@ -626,36 +656,58 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, * * Read image from NAND flash. * Blocks that are marked bad are skipped and the next block is read - * instead as long as the image is short enough to fit even after skipping the - * bad blocks. + * instead as long as the image is short enough to fit even after + * skipping the bad blocks. Due to bad blocks we may not be able to + * perform the requested read. In the case where the read would extend + * beyond the end of the NAND device, both length and actual (if not + * NULL) are set to 0. In the case where the read would extend beyond + * the limit we are passed, length is set to 0 and actual is set to the + * required length. * * @param nand NAND device * @param offset offset in flash * @param length buffer length, on return holds number of read bytes + * @param actual set to size required to read length worth of buffer or 0 + * on error, if not NULL + * @param lim maximum size that actual may be in order to not exceed the + * buffer * @param buffer buffer to write to * @return 0 in case of success */ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, - u_char *buffer) + size_t *actual, loff_t lim, u_char *buffer) { int rval; size_t left_to_read = *length; + size_t used_for_read = 0; u_char *p_buffer = buffer; int need_skip;
if ((offset & (nand->writesize - 1)) != 0) { printf("Attempt to read non page-aligned data\n"); *length = 0; + if (actual) + *actual = 0; return -EINVAL; }
- need_skip = check_skip_len(nand, offset, *length); + need_skip = check_skip_len(nand, offset, *length, &used_for_read); + + if (actual) + *actual = used_for_read; + if (need_skip < 0) { printf("Attempt to read outside the flash area\n"); *length = 0; return -EINVAL; }
+ if (used_for_read > lim) { + puts("Size of read exceeds partition or device limit\n"); + *length = 0; + return -EFBIG; + } + if (!need_skip) { rval = nand_read(nand, offset, length, buffer); if (!rval || rval == -EUCLEAN) diff --git a/include/nand.h b/include/nand.h index dded4e2..f0f3bf9 100644 --- a/include/nand.h +++ b/include/nand.h @@ -129,7 +129,7 @@ struct nand_erase_options { typedef struct nand_erase_options nand_erase_options_t;
int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, - u_char *buffer); + size_t *actual, loff_t lim, u_char *buffer);
#define WITH_YAFFS_OOB (1 << 0) /* whether write with yaffs format. This flag * is a 'mode' meaning it cannot be mixed with @@ -137,7 +137,7 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, #define WITH_DROP_FFS (1 << 1) /* drop trailing all-0xff pages */
int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, - u_char *buffer, int flags); + size_t *actual, loff_t lim, u_char *buffer, int flags); int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts); int nand_torture(nand_info_t *nand, loff_t offset);