Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stop the selSec from being applied twice to lastSector #531

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jpbland1
Copy link
Contributor

ZD 19133

I found a mistake in the update trigger code where the selSec for nvm writeonce was being subtracted from the base address and then subtracted again for the erase operation. We need to add a test with a fullsize update image that would have been corrupted by this mistake. I've previously tried and failed to make a full size image test in test.mk @danielinux I could use advise or help on this

@danielinux
Copy link
Member

danielinux commented Dec 24, 2024

Looks it's the right fix now (edit: tested unsuccessfully. See below). Let's wait for feedback on ZD19133

@danielinux
Copy link
Member

@jpbland1
Here is a manual test to reproduce the issue:

diff --git a/test-app/app_sim.c b/test-app/app_sim.c
index e118e5a9..2cdf8e28 100644
--- a/test-app/app_sim.c
+++ b/test-app/app_sim.c
@@ -112,11 +112,20 @@ int do_cmd(const char *cmd)
     /* wrong command */
     return -1;
 }
+#define FILL_APP_PARTITION
+#ifdef FILL_APP_PARTITION
+#define FILLER_SIZE (105 * 1024)
+static volatile uint8_t filler_data[FILLER_SIZE] = { 0x01, 0x02, 0x03 };
+#endif
+
 
 int main(int argc, char *argv[]) {
 
     int i;
     int ret;
+#ifdef FILL_APP_PARTITION
+    filler_data[FILLER_SIZE - 1] = 0xAA;
+#endif
 
     hal_init();

If you add an arbitrary amount of bytes to the application to grow past the third-last sector, you will see it fail with the NVM workaround. I've used 105KB to go over the limit. With config/examples/sim-nvm-writeonce.config we have sector size 4K and partition size of 0x40000 so the third-last sector starts at 0x40000 - (3 * 0x1000) - 0x100 = 0x3CF00 aka 249600.

To trigger the bug with NVM fix, you should get an application between 249600 and 253695 bytes, e.g.:

$ cp config/examples/sim-nvm-writeonce.config .config
$ make clean && make test-sim-internal-flash-with-update
[...]
Output image(s) successfully created.
	Added        64984 bytes at 0x00000000 from wolfboot.bin
	Added        66088 bytes of 0xff fill
	Added       250216 bytes at 0x00020000 from test-app/image_v1_signed.bin
	Added        11928 bytes of 0xff fill
	Added       250216 bytes at 0x00060000 from test-app/image_v2_signed.bin
	Added        11928 bytes of 0xff fill
	Added         4096 bytes at 0x000a0000 from erased_sec.dd

^ Application size is in the desired range

$ ./tools/scripts/sim-sunnyday-update.sh
Failed update (V: 1)

Update fails with NVM workarounds, succeeds otherwise, or if you make the application smaller than 249600 by reducing the filler size.

Unfortunately, I've tested the current proposed change in this branch and does not seem to fix the issue.

Copy link
Member

@danielinux danielinux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made a quick manual test and it does not seem to fix the reported issue. Please see comments.

@dgarske dgarske removed their request for review December 24, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants