Message ID | 20170623045915.15743-1-cyril.bur@au1.ibm.com |
---|---|
State | Superseded |
Headers | show |
On Fri, 2017-06-23 at 14:59 +1000, Cyril Bur wrote: > A bounds checking mistake prevents opal_flash_{read,write,erase} calls > from having a length equal to the size of the flash. This bug has been > present since the beginning (e7d1f60e core/flash: Add flash API) of > these calls. > > Until before d6a5b53f libflash/blocklevel: Add blocklevel_smart_erase() > 6/4/2017 none of our tools would have performed a single command for the > full size of the flash. It would still have been possible to persuade > `dd` to do this by using a block size equal to the size of the flash > or other mtd related tools. > > Any pflash built with blocklevel_smart_erase() will perform one call to > Linux and then Skiboot for the size of flash. > > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Acked-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> > --- > Slight caveat - when pflash does perform a 64MB erase we spend *a lot* > of time in Skiboot and Linux gets a little bit angry and theres some > RCU stalling all over the place. It does work, so all it well. I am > working on an async bandaid over the problem. > > core/flash.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/core/flash.c b/core/flash.c > index 793401c9..3150df6f 100644 > --- a/core/flash.c > +++ b/core/flash.c > @@ -16,6 +16,7 @@ > > #include <skiboot.h> > #include <cpu.h> > +#include <inttypes.h> > #include <lock.h> > #include <opal.h> > #include <opal-msg.h> > @@ -342,8 +343,10 @@ static int64_t opal_flash_op(enum flash_op op, uint64_t id, uint64_t offset, > goto err; > } > > - if (size >= flash->size || offset >= flash->size > + if (size > flash->size || offset >= flash->size > || offset + size > flash->size) { > + prlog(PR_DEBUG, "Requested flash op %d beyond flash size %" PRIu64 "\n", > + op, flash->size); > rc = OPAL_PARAMETER; > goto err; > }
diff --git a/core/flash.c b/core/flash.c index 793401c9..3150df6f 100644 --- a/core/flash.c +++ b/core/flash.c @@ -16,6 +16,7 @@ #include <skiboot.h> #include <cpu.h> +#include <inttypes.h> #include <lock.h> #include <opal.h> #include <opal-msg.h> @@ -342,8 +343,10 @@ static int64_t opal_flash_op(enum flash_op op, uint64_t id, uint64_t offset, goto err; } - if (size >= flash->size || offset >= flash->size + if (size > flash->size || offset >= flash->size || offset + size > flash->size) { + prlog(PR_DEBUG, "Requested flash op %d beyond flash size %" PRIu64 "\n", + op, flash->size); rc = OPAL_PARAMETER; goto err; }
A bounds checking mistake prevents opal_flash_{read,write,erase} calls from having a length equal to the size of the flash. This bug has been present since the beginning (e7d1f60e core/flash: Add flash API) of these calls. Until before d6a5b53f libflash/blocklevel: Add blocklevel_smart_erase() 6/4/2017 none of our tools would have performed a single command for the full size of the flash. It would still have been possible to persuade `dd` to do this by using a block size equal to the size of the flash or other mtd related tools. Any pflash built with blocklevel_smart_erase() will perform one call to Linux and then Skiboot for the size of flash. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> --- Slight caveat - when pflash does perform a 64MB erase we spend *a lot* of time in Skiboot and Linux gets a little bit angry and theres some RCU stalling all over the place. It does work, so all it well. I am working on an async bandaid over the problem. core/flash.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)