Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/48974 )
Change subject: realtek_mst_i2c_spi.c: Don't depend on int overflows ......................................................................
realtek_mst_i2c_spi.c: Don't depend on int overflows
Be explicit to mask the first byte after the shifts as highlighted by Angel Pos.
BUG=none BRANCH=none TEST=builds
Change-Id: I7d1215678094d709e79b8f8c96aa3810586cd72e Signed-off-by: Edward O'Callaghan quasisec@google.com Spotted-by: Angel Pons th3fanbus@gmail.com --- M realtek_mst_i2c_spi.c 1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/48974/1
diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index dabec70..3e84b23 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -330,9 +330,9 @@ start--; ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x46); // ** ret |= realtek_mst_i2c_spi_write_register(fd, 0x61, OPCODE_READ); - uint8_t block_idx = start >> 16; - uint8_t page_idx = start >> 8; - uint8_t byte_idx = start; + uint8_t block_idx = (start >> 16) & 0xff; + uint8_t page_idx = (start >> 8) & 0xff; + uint8_t byte_idx = start & 0xff; ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx); ret |= realtek_mst_i2c_spi_write_register(fd, 0x6a, 0x03); ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x47); // ** @@ -384,9 +384,9 @@ uint16_t page_len = min(len - i, PAGE_SIZE); if (len - i < PAGE_SIZE) ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, page_len-1); - uint8_t block_idx = (start + i) >> 16; - uint8_t page_idx = (start + i) >> 8; - uint8_t byte_idx = start + i; + uint8_t block_idx = ((start + i) >> 16) & 0xff; + uint8_t page_idx = ((start + i) >> 8) & 0xff; + uint8_t byte_idx = (start + i) & 0xff; ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx); if (ret) break;
Shiyu Sun has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48974 )
Change subject: realtek_mst_i2c_spi.c: Don't depend on int overflows ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48974 )
Change subject: realtek_mst_i2c_spi.c: Don't depend on int overflows ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/flashrom/+/48974/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/48974/1//COMMIT_MSG@10 PS1, Line 10: Pos Po*n*s
Hello build bot (Jenkins), Shiyu Sun, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/48974
to look at the new patch set (#2).
Change subject: realtek_mst_i2c_spi.c: Don't depend on int overflows ......................................................................
realtek_mst_i2c_spi.c: Don't depend on int overflows
Be explicit to mask the first byte after the shifts as highlighted by Angel Pons.
BUG=none BRANCH=none TEST=builds
Change-Id: I7d1215678094d709e79b8f8c96aa3810586cd72e Signed-off-by: Edward O'Callaghan quasisec@google.com Spotted-by: Angel Pons th3fanbus@gmail.com --- M realtek_mst_i2c_spi.c 1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/48974/2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48974 )
Change subject: realtek_mst_i2c_spi.c: Don't depend on int overflows ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/flashrom/+/48974/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/48974/1//COMMIT_MSG@10 PS1, Line 10: Pos
Po*n*s
Done
Hello build bot (Jenkins), Shiyu Sun, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/48974
to look at the new patch set (#3).
Change subject: realtek_mst_i2c_spi.c: Don't depend on int overflows ......................................................................
realtek_mst_i2c_spi.c: Don't depend on int overflows
Be explicit to mask the first byte after the shifts as highlighted by Angel Pons.
BUG=none BRANCH=none TEST=builds
Change-Id: I7d1215678094d709e79b8f8c96aa3810586cd72e Signed-off-by: Edward O'Callaghan quasisec@google.com Spotted-by: Angel Pons th3fanbus@gmail.com --- M realtek_mst_i2c_spi.c 1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/48974/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48974 )
Change subject: realtek_mst_i2c_spi.c: Don't depend on int overflows ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/flashrom/+/48974/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/48974/1//COMMIT_MSG@10 PS1, Line 10: Pos
Done
Thanks!
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/48974 )
Change subject: realtek_mst_i2c_spi.c: Don't depend on int overflows ......................................................................
realtek_mst_i2c_spi.c: Don't depend on int overflows
Be explicit to mask the first byte after the shifts as highlighted by Angel Pons.
BUG=none BRANCH=none TEST=builds
Change-Id: I7d1215678094d709e79b8f8c96aa3810586cd72e Signed-off-by: Edward O'Callaghan quasisec@google.com Spotted-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/48974 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Shiyu Sun sshiyu@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M realtek_mst_i2c_spi.c 1 file changed, 6 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Shiyu Sun: Looks good to me, but someone else must approve
diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index dabec70..3e84b23 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -330,9 +330,9 @@ start--; ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x46); // ** ret |= realtek_mst_i2c_spi_write_register(fd, 0x61, OPCODE_READ); - uint8_t block_idx = start >> 16; - uint8_t page_idx = start >> 8; - uint8_t byte_idx = start; + uint8_t block_idx = (start >> 16) & 0xff; + uint8_t page_idx = (start >> 8) & 0xff; + uint8_t byte_idx = start & 0xff; ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx); ret |= realtek_mst_i2c_spi_write_register(fd, 0x6a, 0x03); ret |= realtek_mst_i2c_spi_write_register(fd, 0x60, 0x47); // ** @@ -384,9 +384,9 @@ uint16_t page_len = min(len - i, PAGE_SIZE); if (len - i < PAGE_SIZE) ret |= realtek_mst_i2c_spi_write_register(fd, 0x71, page_len-1); - uint8_t block_idx = (start + i) >> 16; - uint8_t page_idx = (start + i) >> 8; - uint8_t byte_idx = start + i; + uint8_t block_idx = ((start + i) >> 16) & 0xff; + uint8_t page_idx = ((start + i) >> 8) & 0xff; + uint8_t byte_idx = (start + i) & 0xff; ret |= realtek_mst_i2c_spi_map_page(fd, block_idx, page_idx, byte_idx); if (ret) break;
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Shiyu Sun (Code Review)