| Message ID | 1434120071-4905-1-git-send-email-clg@fr.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Cédric, > > - if (size >= flash->size || offset >= flash->size > - || offset + size >= flash->size) { > + if (offset + size > flash->size) { > rc = OPAL_PARAMETER; > goto err; > } This loses the check for the overflow condition (where offset + size wraps), which we got from flash->size being a u32. How about we just make the minimal fix: || offset + size > flash->size ? Or, make the overflow check explicit. Cheers, Jeremy
On 06/13/2015 06:42 AM, Jeremy Kerr wrote: > Hi Cédric, > >> >> - if (size >= flash->size || offset >= flash->size >> - || offset + size >= flash->size) { >> + if (offset + size > flash->size) { >> rc = OPAL_PARAMETER; >> goto err; >> } > > This loses the check for the overflow condition (where offset + size wraps), which we got from flash->size being a u32. How about we just make the minimal fix: > > || offset + size > flash->size > > ? Will do that. V2 should arrive soon. Thanks, C. > > Or, make the overflow check explicit. > > Cheers, > > > Jeremy > >
Index: skiboot.git/core/flash.c =================================================================== --- skiboot.git.orig/core/flash.c +++ skiboot.git/core/flash.c @@ -311,8 +311,7 @@ static int64_t opal_flash_op(enum flash_ goto err; } - if (size >= flash->size || offset >= flash->size - || offset + size >= flash->size) { + if (offset + size > flash->size) { rc = OPAL_PARAMETER; goto err; }
Copying the flash from the host fails : # cat /dev/mtd0 > pnor cat: /dev/mtd0: Input/output error and the kernel logs : [ 1357.866996] mtd mtd0: opal_flash_async_op(op=0) failed (rc -1) It seems that the check on the parameters in the opal_flash_op() routine are bit excessive and we fail to write or read the last block. Here is a fix below which should be enough to catch an out of bounds operation. Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> --- core/flash.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)