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.