B0 self lock before jump #20110

michalek-no · 2025-01-29T12:05:27Z

Disables read and execute on memory containing NSIB
right before jumping to application.

NordicBuilder · 2025-01-29T12:08:50Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 11

Inputs:

Sources:

sdk-nrf: PR head: e342f8a184de2af57405a21dbc112297e0f0f29f

more details

sdk-nrf:

PR head: e342f8a184de2af57405a21dbc112297e0f0f29f
merge base: 436fd2f18f54ee032a18ab8693362de19310f8ff
target head (main): 2116d91350901ecd55b7a3f4d1f4fe70a9dd2cbc
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (2)

subsys
│  ├── bootloader
│  │  ├── Kconfig
│  │  ├── bl_boot
│  │  │  │ bl_boot.c

Outputs:

Toolchain

Version: 342151af73
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:342151af73_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain - Skipped: existing toolchain is used
❌ Build twister
- sdk-nrf test count: 541
❌ Integration tests
- ❌ test-fw-nrfconnect-boot
- ❌ test-fw-nrfconnect-chip
- ❌ test-fw-nrfconnect-tfm
- ❌ test-fw-nrfconnect-zigbee
- ❌ test-sdk-find-my
- ❌ test-sdk-sidewalk
- ❌ test-sdk-mcuboot
- ⚠️ test-fw-nrfconnect-fw-update
- ⚠️ test-fw-nrfconnect-nrf-iot_cloud

Disabled integration tests

- desktop52_verification
- doc-internal
- test_ble_nrf_config
- test-fw-nrfconnect-apps
- test-fw-nrfconnect-ble_mesh
- test-fw-nrfconnect-ble_samples
- test-fw-nrfconnect-fem
- test-fw-nrfconnect-nfc
- test-fw-nrfconnect-nrf-iot_lwm2m
- test-fw-nrfconnect-nrf-iot_mosh
- test-fw-nrfconnect-nrf-iot_positioning
- test-fw-nrfconnect-nrf-iot_samples
- test-fw-nrfconnect-nrf-iot_serial_lte_modem
- test-fw-nrfconnect-nrf-iot_thingy91
- test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- test-fw-nrfconnect-nrf_crypto
- test-fw-nrfconnect-ps
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-rs
- test-fw-nrfconnect-thread
- test-low-level
- test-sdk-audio
- test-sdk-dfu
- test-sdk-pmic-samples
- test-sdk-wifi
- test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2025-01-29T12:25:28Z

You can find the documentation preview for this PR at this link.

Note: This comment is automatically posted by the Documentation Publish GitHub Action.

michalek-no · 2025-01-30T08:26:34Z

rebase

subsys/bootloader/bl_boot/bl_boot.c

nvlsianpu · 2025-01-31T17:16:47Z

subsys/bootloader/bl_boot/bl_boot.c

 		:
 		: "r" (vector_table[1]), "i" (CONFIG_SRAM_BASE_ADDRESS),
 		  "i" (CONFIG_SRAM_SIZE * 1024), "i" (0)
 		: "r0", "r1", "r2", "r3", "memory"
 	);
+#elif defined(CONFIG_SB_DISABLE_SELF_R_X)
+
+	relocate_bootconf_disable_r_x_and_jump()(vector_table[1]);


strange syntax. Is that r0 might contain value of reset_vector, if not (vector_table[1]) does the job of populating r0?
I would rather create two alternatives of relocate_bootconf_disable_r_x_and_jump() function.

Yeah, that might be a problem since at the point of execution of the relocate_bootconf_disable_r_x_and_jump, the vector_table, which is not constant, has already been wiped.
I think the relocate_bootconf_disable_r_x_and_jump should combine both the cleanup and lock.

Good catch.

it returns pointer to the function in ram that is being called in place with vector_table

nvlsianpu · 2025-01-31T17:23:06Z

subsys/bootloader/bl_boot/bl_boot.c

+	);
+}
+
+static ram_fn_ptr relocate_bootconf_disable_r_x_and_jump(void)


how this function preserves r0 value?

this function returns pointer to the callable one

de-nordic · 2025-01-31T18:39:20Z

subsys/bootloader/bl_boot/bl_boot.c

@@ -174,13 +223,18 @@ void bl_boot(const struct fw_info *fw_info)
 		"   b   clear\n"
 		"out:\n"
 		"   dsb\n"
+#ifndef CONFIG_SB_DISABLE_SELF_R_X


If you disable the jump to vector, r0 is not needed here so the line 211 can be commented out and in line 233 the r0 does not have to be on list. I suspect that impact on size is not that great, but code will get uglier.

nordicjm · 2025-02-03T10:27:19Z

subsys/bootloader/bl_boot/bl_boot.c

+#define FUNCTION_BUFFER_LEN 128
+#define RRAMC_REGION_RWX_LSB 0
+#define RRAMC_REGION_RWX_WIDTH 3
+#define RRAMC_REGION_TO_LOCK_ADDR 0x5004b56c


should get this address from dts then offset using the nrfx hal offset define

nordicjm · 2025-02-03T10:27:28Z

subsys/bootloader/bl_boot/bl_boot.c

+#define RRAMC_REGION_TO_LOCK_ADDR_H (RRAMC_REGION_TO_LOCK_ADDR >> 16)
+#define RRAMC_REGION_TO_LOCK_ADDR_L (RRAMC_REGION_TO_LOCK_ADDR & 0x0000fffful)


would assume these are in that file too

nordicjm · 2025-02-03T10:27:32Z

subsys/bootloader/bl_boot/bl_boot.c

+#define RRAMC_REGION_TO_LOCK_ADDR_H (RRAMC_REGION_TO_LOCK_ADDR >> 16)
+#define RRAMC_REGION_TO_LOCK_ADDR_L (RRAMC_REGION_TO_LOCK_ADDR & 0x0000fffful)
+typedef void (*ram_fn_ptr)(uint32_t);
+uint8_t ram_exec_buf[FUNCTION_BUFFER_LEN];


nordicjm · 2025-02-03T10:31:47Z

subsys/bootloader/bl_boot/bl_boot.c

+		"   MOV r5, %0\n"
+		"   MOV r1, %1\n"
+		"   MOVT r1, %2\n"
+		"   LDR r0, [r1]\n"
+		/* Size of the region should be set at this point
+		 * by provisioning through BOOTCONF. If not set it according partition size.
+		 */
+		"   RORS r4, r0, #16\n"
+		"   CBNZ r4, clear_rwx\n"
+		"   MOVT r0, %3\n"
+		"clear_rwx:\n"
+		"   BFC r0, %4, %5\n"
+		"   STR r0, [r1]\n"
+		"   DSB\n"
+		"   BX r5\n"


lowercase like below code does

nordicjm · 2025-02-03T10:32:14Z

subsys/bootloader/bl_boot/bl_boot.c

+		: "r" (vector), "i" (RRAMC_REGION_TO_LOCK_ADDR_L),
+		  "i" (RRAMC_REGION_TO_LOCK_ADDR_H), "i" (CONFIG_PM_PARTITION_SIZE_B0_IMAGE / 1024),
+		  "i" (RRAMC_REGION_RWX_LSB), "i" (RRAMC_REGION_RWX_WIDTH)
+		: "r0", "r1", "r2", "r4", "r5", "memory"


r2 doesn't look to be used

nordicjm · 2025-02-03T10:34:06Z

subsys/bootloader/bl_boot/bl_boot.c

+#endif /* CONFIG_SB_CLEANUP_RAM */
+
+#if defined(CONFIG_SB_DISABLE_SELF_R_X)
+	relocate_bootconf_disable_r_x_and_jump()(vector_table[1]);


don't think this will work, at this point the RAM is blank, and you are returning to C-land where it expects to have a fully working stack pointer including all data on stack. Would just make more sense to have one asm block with #ifdefs in it to select which parts are enabled or not, without calling C functions in-between

Disables read and execute on memory containing NSIB right before jumping to application. Signed-off-by: Mateusz Michalek <[email protected]>

michalek-no requested review from nvlsianpu, fundakol, nordicjm, de-nordic and gchwier January 29, 2025 12:05

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 29, 2025

michalek-no force-pushed the mb-b0-self-lock branch 2 times, most recently from b064fa9 to 056c521 Compare January 30, 2025 08:17

michalek-no changed the title ~~wip B0 self lock before jump~~ B0 self lock before jump Jan 30, 2025

michalek-no marked this pull request as ready for review January 30, 2025 08:19

michalek-no requested review from a team as code owners January 30, 2025 08:19

michalek-no force-pushed the mb-b0-self-lock branch 2 times, most recently from 2abe768 to 60d24dd Compare January 30, 2025 08:26

michalek-no force-pushed the mb-b0-self-lock branch 4 times, most recently from cf1c54a to e12aa78 Compare January 30, 2025 13:16

nvlsianpu reviewed Jan 31, 2025

View reviewed changes

de-nordic reviewed Jan 31, 2025

View reviewed changes

michalek-no force-pushed the mb-b0-self-lock branch 2 times, most recently from dcbbcee to b265d74 Compare February 3, 2025 09:27

nordicjm reviewed Feb 3, 2025

View reviewed changes

bootloader: self lock at the end of execution

e342f8a

Disables read and execute on memory containing NSIB right before jumping to application. Signed-off-by: Mateusz Michalek <[email protected]>

michalek-no force-pushed the mb-b0-self-lock branch from b265d74 to e342f8a Compare February 3, 2025 19:48

michalek-no added the DNM label Feb 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

B0 self lock before jump #20110

B0 self lock before jump #20110

michalek-no commented Jan 29, 2025 •

edited

Loading

NordicBuilder commented Jan 29, 2025 •

edited

Loading

NordicBuilder commented Jan 29, 2025

michalek-no commented Jan 30, 2025

nvlsianpu Jan 31, 2025

de-nordic Jan 31, 2025

de-nordic Jan 31, 2025

michalek-no Feb 3, 2025

nvlsianpu Jan 31, 2025

michalek-no Feb 3, 2025

de-nordic Jan 31, 2025

nordicjm Feb 3, 2025

nordicjm Feb 3, 2025

nordicjm Feb 3, 2025

nordicjm Feb 3, 2025

nordicjm Feb 3, 2025

nordicjm Feb 3, 2025

		#define RRAMC_REGION_TO_LOCK_ADDR_H (RRAMC_REGION_TO_LOCK_ADDR >> 16)
		#define RRAMC_REGION_TO_LOCK_ADDR_L (RRAMC_REGION_TO_LOCK_ADDR & 0x0000fffful)

B0 self lock before jump #20110

Are you sure you want to change the base?

B0 self lock before jump #20110

Conversation

michalek-no commented Jan 29, 2025 • edited Loading

NordicBuilder commented Jan 29, 2025 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

NordicBuilder commented Jan 29, 2025

michalek-no commented Jan 30, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalek-no commented Jan 29, 2025 •

edited

Loading

NordicBuilder commented Jan 29, 2025 •

edited

Loading