Patchwork [U-Boot,v2,1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters

login
register
mail settings
Submitter Tom Rini
Date Feb. 26, 2013, 3:56 p.m.
Message ID <1361894171-3379-2-git-send-email-trini@ti.com>
Download mbox | patch
Permalink /patch/223255/
State Superseded
Delegated to: Tom Rini
Headers show

Comments

Tom Rini - Feb. 26, 2013, 3:56 p.m.
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 care about total actual size used rather than block_size
chunks used.  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.

Cc: Scott Wood <scottwood@freescale.com>
Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Tom Rini <trini@ti.com>
---
 common/cmd_nand.c            |   61 +++++++++++++++++++++--------------------
 common/env_nand.c            |    5 ++--
 drivers/mtd/nand/nand_util.c |   62 +++++++++++++++++++++++++++++++-----------
 include/nand.h               |    4 +--
 4 files changed, 82 insertions(+), 50 deletions(-)
Tom Rini - Feb. 27, 2013, 4:49 p.m.
On Tue, Feb 26, 2013 at 10:56:08AM -0500, Tom Rini wrote:

> 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
[snip]
>  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)
[snip]
> +	if (*actual > lim) {
> +		puts("Size of read exceeds partition or device limit\n");
> +		*length = 0;
> +		return -EFBIG;
> +	}

The more I look at this and try testing things, I think I shouldn't be
introducing a change here.  Before you could do:
nand read ${address} partname-with-badblock

And it would suceed but bleed into the next partition if it wasn't the
last one.  So your production system could do "nand read ${address}
kernel" and be OK.  But with this change, it would fail because reading
the whole partition is now too large with a bad block (you would need
partition+(blocksize*number bad blocks).

So I'm going to put this back to a check simply against requested size
being greater than lim rather than required size greater than lim (since
required size exceeds device is still caught).
Scott Wood - Feb. 27, 2013, 5:09 p.m.
On 02/27/2013 10:49:55 AM, Tom Rini wrote:
> On Tue, Feb 26, 2013 at 10:56:08AM -0500, Tom Rini wrote:
> 
> > 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
> [snip]
> >  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)
> [snip]
> > +	if (*actual > lim) {
> > +		puts("Size of read exceeds partition or device  
> limit\n");
> > +		*length = 0;
> > +		return -EFBIG;
> > +	}
> 
> The more I look at this and try testing things, I think I shouldn't be
> introducing a change here.  Before you could do:
> nand read ${address} partname-with-badblock
> 
> And it would suceed but bleed into the next partition if it wasn't the
> last one.  So your production system could do "nand read ${address}
> kernel" and be OK.  But with this change, it would fail because  
> reading
> the whole partition is now too large with a bad block (you would need
> partition+(blocksize*number bad blocks).

You wouldn't be quite so OK if it were a write instead.

> So I'm going to put this back to a check simply against requested size
> being greater than lim rather than required size greater than lim  
> (since
> required size exceeds device is still caught).

No, please retain the check.  The other issue is a separate (though  
related) bug, and there's a patch from Harvey Chapman to deal with it.

-Scott
Tom Rini - Feb. 27, 2013, 5:17 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/27/2013 12:09 PM, Scott Wood wrote:
> On 02/27/2013 10:49:55 AM, Tom Rini wrote:
>> On Tue, Feb 26, 2013 at 10:56:08AM -0500, Tom Rini wrote:
>> 
>>> 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
>> [snip]
>>> 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)
>> [snip]
>>> +    if (*actual > lim) { +        puts("Size of read exceeds
>>> partition or device limit\n"); +        *length = 0; +
>>> return -EFBIG; +    }
>> 
>> The more I look at this and try testing things, I think I
>> shouldn't be introducing a change here.  Before you could do: 
>> nand read ${address} partname-with-badblock
>> 
>> And it would suceed but bleed into the next partition if it
>> wasn't the last one.  So your production system could do "nand
>> read ${address} kernel" and be OK.  But with this change, it
>> would fail because reading the whole partition is now too large
>> with a bad block (you would need partition+(blocksize*number bad
>> blocks).
> 
> You wouldn't be quite so OK if it were a write instead.

Correct.  But the check is now inside of nand_(read|write)_skip_bad.
So the write case will fail (without trying to write), but the read
case should not.

>> So I'm going to put this back to a check simply against requested
>> size being greater than lim rather than required size greater
>> than lim (since required size exceeds device is still caught).
> 
> No, please retain the check.  The other issue is a separate
> (though related) bug, and there's a patch from Harvey Chapman to
> deal with it.

I could be missing something, but I'm not sure how to otherwise adjust
things here.  With all of the checking moved to nand_util.c and
check_skip_len not knowing if we're a read or write it can only say
"fits using X" or "exceeds device", then it's up to the caller to
decide the next action.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRLj+TAAoJENk4IS6UOR1WJAAP/RLxsGHN4HqoEC0zfe2eIDZe
ZJU+sevT/murom6fNOfKFuC8pwPavy64FGdXNspwdpJCRTcXIs503k4x4ZMy9USC
HszV9NKF42HyscQi3RmBqlIUb9WQ6UikMZEUpteO0ZGQajRKho3nL3M1qjQsASG9
745qrmJijQEjEu3MsnfEZYEzyliVCpHHiIylhBALizwpjEwzKwlebe/ZrmbTPYO6
QXyfHLd+/5iNUJEjXOaXP/z7gXU+85m2uwxRhyLB1PmULGkmh7A+cvByTPt8TcxK
HTWnL3at4S+OvWqlnYTdTYbsCBUQ3jp/wnLF39vq9E5VIPv7UshwPponeNbFEGzt
GOZP1fIGMWcSFJHd3usR7wkhA5tJZu9329Av283ckwIbSlwz/tOP/efwFF/Zsrc5
oLn4ezRrA+RSKCtCujYJPhnAyEJEkncvpA4Imx6R9v9LZlXu3z8w5AQKcuAlsrxm
FDcjtcAA6y804I2jGPvL9laoCy9li1SVMHbd71zVtRwRJBMYUMfw7aI6uUwBslLJ
4GlnVKKsfEAkEb7APZ5v5YJrwD+mgSJ4TR1fwtoirbQKpG3OrMnXNhzZSBD5UFv6
DUxhMRL+EOawFJIl5AUPHHB7LdvV4qtDwee1bPg31ZXetnimQGLHn8qzppioMIvC
suvFme3/03ZGTDFwUUjp
=My8j
-----END PGP SIGNATURE-----
Tom Rini - Feb. 27, 2013, 5:22 p.m.
On Wed, Feb 27, 2013 at 12:17:07PM -0500, Tom Rini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 02/27/2013 12:09 PM, Scott Wood wrote:
> > On 02/27/2013 10:49:55 AM, Tom Rini wrote:
> >> On Tue, Feb 26, 2013 at 10:56:08AM -0500, Tom Rini wrote:
> >> 
> >>> 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
> >> [snip]
> >>> 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)
> >> [snip]
> >>> +    if (*actual > lim) { +        puts("Size of read exceeds
> >>> partition or device limit\n"); +        *length = 0; +
> >>> return -EFBIG; +    }
> >> 
> >> The more I look at this and try testing things, I think I
> >> shouldn't be introducing a change here.  Before you could do: 
> >> nand read ${address} partname-with-badblock
> >> 
> >> And it would suceed but bleed into the next partition if it
> >> wasn't the last one.  So your production system could do "nand
> >> read ${address} kernel" and be OK.  But with this change, it
> >> would fail because reading the whole partition is now too large
> >> with a bad block (you would need partition+(blocksize*number bad
> >> blocks).
> > 
> > You wouldn't be quite so OK if it were a write instead.
> 
> Correct.  But the check is now inside of nand_(read|write)_skip_bad.
> So the write case will fail (without trying to write), but the read
> case should not.
> 
> >> So I'm going to put this back to a check simply against requested
> >> size being greater than lim rather than required size greater
> >> than lim (since required size exceeds device is still caught).
> > 
> > No, please retain the check.  The other issue is a separate
> > (though related) bug, and there's a patch from Harvey Chapman to
> > deal with it.
> 
> I could be missing something, but I'm not sure how to otherwise adjust
> things here.  With all of the checking moved to nand_util.c and
> check_skip_len not knowing if we're a read or write it can only say
> "fits using X" or "exceeds device", then it's up to the caller to
> decide the next action.

OK, I see it now.  Yes, I think they should be able to live together
nicely.

Patch

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 1568594..e091e02 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->offset + 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");
@@ -188,36 +191,29 @@  static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *maxsize)
 }
 
 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) {
-		puts("Size exceeds partition or device limit\n");
-		return -1;
-	}
-
 print:
 	printf("device %d ", *idx);
 	if (*size == nand_info[*idx].size)
@@ -299,15 +295,14 @@  int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
 
 		printf("0x%08lx\n", nand_env_oob_offset);
 	} else if (!strcmp(cmd, "set")) {
-		loff_t addr;
-		loff_t maxsize;
+		loff_t addr, size, maxsize;
 		struct mtd_oob_ops ops;
 		int idx = 0;
 
 		if (argc < 3)
 			goto usage;
 
-		if (arg_off(argv[2], &idx, &addr, &maxsize)) {
+		if (arg_off(argv[2], &idx, &addr, &size, &maxsize)) {
 			puts("Offset or partition name expected\n");
 			return 1;
 		}
@@ -432,7 +427,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
@@ -557,7 +552,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];
@@ -605,7 +601,7 @@  static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	}
 
 	if (strncmp(cmd, "read", 4) == 0 || strncmp(cmd, "write", 5) == 0) {
-		size_t rwsize;
+		size_t rwsize, actual;
 		ulong pagecount = 1;
 		int read;
 		int raw;
@@ -625,7 +621,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)) {
@@ -641,7 +637,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;
@@ -651,9 +647,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,
+							 &actual, maxsize,
 							 (u_char *)addr);
 			else
 				ret = nand_write_skip_bad(nand, off, &rwsize,
+							  &actual, maxsize,
 							  (u_char *)addr, 0);
 #ifdef CONFIG_CMD_NAND_TRIMFFS
 		} else if (!strcmp(s, ".trimffs")) {
@@ -661,8 +659,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, &actual,
+						maxsize, (u_char *)addr,
 						WITH_DROP_FFS);
 #endif
 #ifdef CONFIG_CMD_NAND_YAFFS
@@ -671,8 +669,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, &actual,
+						maxsize, (u_char *)addr,
 						WITH_INLINE_OOB);
 #endif
 		} else if (!strcmp(s, ".oob")) {
@@ -781,7 +779,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)) {
@@ -862,7 +861,7 @@  static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
 {
 	int r;
 	char *s;
-	size_t cnt;
+	size_t cnt, actual;
 	image_header_t *hdr;
 #if defined(CONFIG_FIT)
 	const void *fit_hdr = NULL;
@@ -879,7 +878,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, &actual, nand->size,
+			(u_char *) addr);
 	if (r) {
 		puts("** Read error\n");
 		bootstage_error(BOOTSTAGE_ID_NAND_HDR_READ);
@@ -911,7 +911,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, &actual, 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 22e72a2..05efbf5 100644
--- a/common/env_nand.c
+++ b/common/env_nand.c
@@ -266,7 +266,7 @@  int readenv(size_t offset, u_char *buf)
 {
 	size_t end = offset + CONFIG_ENV_RANGE;
 	size_t amount_loaded = 0;
-	size_t blocksize, len;
+	size_t blocksize, len, actual;
 	u_char *char_ptr;
 
 	blocksize = nand_info[0].erasesize;
@@ -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, &actual,
+					       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 2ba0c5e..5ed5b1d 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -397,34 +397,38 @@  int nand_unlock(struct mtd_info *mtd, loff_t start, size_t length,
  * blocks fits into device
  *
  * @param nand NAND device
- * @param offset offset in flash
+ * @param offsetp offset in flash (on exit offset where it's ending)
  * @param length image 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 len_excl_bad = 0;
 	int ret = 0;
 
-	while (len_excl_bad < length) {
+	while (length > 0) {
 		size_t block_len, block_off;
 		loff_t block_start;
 
-		if (offset >= nand->size)
+		if (*offset >= nand->size)
 			return -1;
 
-		block_start = offset & ~(loff_t)(nand->erasesize - 1);
-		block_off = offset & (nand->erasesize - 1);
+		block_start = *offset & ~(loff_t)(nand->erasesize - 1);
+		block_off = *offset & (nand->erasesize - 1);
 		block_len = nand->erasesize - block_off;
 
-		if (!nand_block_isbad(nand, block_start))
-			len_excl_bad += block_len;
-		else
+		if (!nand_block_isbad(nand, block_start)) {
+			if (block_len > length) {
+				/* Final chunk is smaller than block. */
+				*offset += length;
+				return ret;
+			} else
+				length -= block_len;
+		} else
 			ret = 1;
 
-		offset += block_len;
+		*offset += block_len;
 	}
 
 	return ret;
@@ -459,22 +463,26 @@  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.  Note that the actual size needed may exceed
+ * both the length and available NAND due to bad blocks.
  *
  * @param nand  	NAND device
  * @param offset	offset in flash
  * @param length	buffer length
+ * @param actual	size required to write length worth of buffer
+ * @param lim		end location of where data in the buffer may be written.
  * @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;
 	u_char *p_buffer = buffer;
 	int need_skip;
+	loff_t tmp_offset;
 
 #ifdef CONFIG_CMD_NAND_YAFFS
 	if (flags & WITH_YAFFS_OOB) {
@@ -509,16 +517,25 @@  int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
 	if ((offset & (nand->writesize - 1)) != 0) {
 		printf("Attempt to write non page-aligned data\n");
 		*length = 0;
+		*actual = 0;
 		return -EINVAL;
 	}
 
-	need_skip = check_skip_len(nand, offset, *length);
+	tmp_offset = offset;
+	need_skip = check_skip_len(nand, &tmp_offset, *length);
+	*actual = tmp_offset;
 	if (need_skip < 0) {
 		printf("Attempt to write outside the flash area\n");
 		*length = 0;
 		return -EINVAL;
 	}
 
+	if (*actual > 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)
@@ -610,35 +627,48 @@  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.  Note that the actual size needed may exceed the length due to
  * bad blocks.
  *
  * @param nand NAND device
  * @param offset offset in flash
  * @param length buffer length, on return holds number of read bytes
+ * @param actual size required to read length worth of buffer
+ * @param lim end location of where data in the buffer may be written.
  * @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;
 	u_char *p_buffer = buffer;
 	int need_skip;
+	loff_t tmp_offset;
 
 	if ((offset & (nand->writesize - 1)) != 0) {
 		printf("Attempt to read non page-aligned data\n");
 		*length = 0;
+		*actual = 0;
 		return -EINVAL;
 	}
 
-	need_skip = check_skip_len(nand, offset, *length);
+	tmp_offset = offset;
+	need_skip = check_skip_len(nand, &tmp_offset, *length);
+	*actual = tmp_offset;
 	if (need_skip < 0) {
 		printf("Attempt to read outside the flash area\n");
 		*length = 0;
 		return -EINVAL;
 	}
 
+	if (*actual > 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);