diff mbox series

[U-Boot] tools: omapimage: fix corner-case in byteswap path

Message ID 1512403442-11926-1-git-send-email-philipp.tomsich@theobroma-systems.com
State Accepted
Commit c8e1ca3ebfd21915f6f2e399c9ca1cd3d7a4b076
Delegated to: Tom Rini
Headers show
Series [U-Boot] tools: omapimage: fix corner-case in byteswap path | expand

Commit Message

Philipp Tomsich Dec. 4, 2017, 4:04 p.m. UTC
Since commit 2614a208471e ("common: command: tempory buffer should
have size of command line buf"), there have been consistent Travis CI
failures on my builds (interestingly not for Tom, even though building
the same commit id) due to a SEGV in building the byteswapped
omapimage:
     	    arm: pcm051_rev3
     make[2]: *** [MLO.byteswap] Error 139
     	      	  		       ^^^ error code for a SEGV

Turns out that the word-based byte-swapping loop in omapimage.c is to
blame. With the loop condition
       while (swapped <= (sbuf->st_size / sizeof(uint32_t)))
there had been one-too-many iterations for all file sizes divisible by
the sizeof(uint32_t).  I.e. we had 1 iteration for 0 bytes (and also 1
through 3 bytes) and 2 iterations at 4 bytes... clearly overshooting
on 0 and 4 bytes.

This commit fixes the calculation of an up-rounded word-count and
makes sure to keep the zero-based loop-counter below the number of
words to be processed.

References: 2614a20 ("common: command: tempory buffer should have size of command line buf")
Fixes: 79b9ebb ("omapimage: Add support for byteswapped SPI images")
Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Reviewed-by: Martin Elshuber <martin.elshuber@theobroma-systems.com>

---

 tools/omapimage.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Tom Rini Dec. 7, 2017, 1:21 a.m. UTC | #1
On Mon, Dec 04, 2017 at 05:04:02PM +0100, Philipp Tomsich wrote:

> Since commit 2614a208471e ("common: command: tempory buffer should
> have size of command line buf"), there have been consistent Travis CI
> failures on my builds (interestingly not for Tom, even though building
> the same commit id) due to a SEGV in building the byteswapped
> omapimage:
>      	    arm: pcm051_rev3
>      make[2]: *** [MLO.byteswap] Error 139
>      	      	  		       ^^^ error code for a SEGV
> 
> Turns out that the word-based byte-swapping loop in omapimage.c is to
> blame. With the loop condition
>        while (swapped <= (sbuf->st_size / sizeof(uint32_t)))
> there had been one-too-many iterations for all file sizes divisible by
> the sizeof(uint32_t).  I.e. we had 1 iteration for 0 bytes (and also 1
> through 3 bytes) and 2 iterations at 4 bytes... clearly overshooting
> on 0 and 4 bytes.
> 
> This commit fixes the calculation of an up-rounded word-count and
> makes sure to keep the zero-based loop-counter below the number of
> words to be processed.
> 
> References: 2614a20 ("common: command: tempory buffer should have size of command line buf")
> Fixes: 79b9ebb ("omapimage: Add support for byteswapped SPI images")
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Martin Elshuber <martin.elshuber@theobroma-systems.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/tools/omapimage.c b/tools/omapimage.c
index e31b94a..e7c4638 100644
--- a/tools/omapimage.c
+++ b/tools/omapimage.c
@@ -20,6 +20,8 @@ 
 #include "gpheader.h"
 #include "omapimage.h"
 
+#define DIV_ROUND_UP(n, d)     (((n) + (d) - 1) / (d))
+
 /* Header size is CH header rounded up to 512 bytes plus GP header */
 #define OMAP_CH_HDR_SIZE 512
 #define OMAP_FILE_HDR_SIZE (OMAP_CH_HDR_SIZE + GPIMAGE_HDR_SIZE)
@@ -150,8 +152,10 @@  static void omapimage_set_header(void *ptr, struct stat *sbuf, int ifd,
 		do_swap32 = 1;
 		int swapped = 0;
 		uint32_t *data = (uint32_t *)ptr;
+		const off_t size_in_words =
+			DIV_ROUND_UP(sbuf->st_size, sizeof(uint32_t));
 
-		while (swapped <= (sbuf->st_size / sizeof(uint32_t))) {
+		while (swapped < size_in_words) {
 			*data = cpu_to_be32(*data);
 			swapped++;
 			data++;