samples: matter: Moved confirmation of the OTA image to the Init.

Confirmation of the OTA update should not be delegated to be
done in Matter Thread because Fprotect already has been set
during the factory data initialization and blocked writing
to the restricted areas. Confirming must be done in the
Init method before initializing the Factory data module.
Then, the result should be delivered to the image processor
implementation.

Signed-off-by: Arkadiusz Balys <arkadiusz.balys@nordicsemi.no>

applications/matter_bridge/src/app_task.cpp

@@ -131,6 +131,14 @@ CHIP_ERROR AppTask::Init()
 	k_timer_init(&sFunctionTimer, &AppTask::FunctionTimerTimeoutCallback, nullptr);
 	k_timer_user_data_set(&sFunctionTimer, this);
 
+#ifdef CONFIG_CHIP_OTA_REQUESTOR
+	/* OTA image confirmation must be done before the factory data init. */
+	err = OtaConfirmNewImage();
+	if (err != CHIP_NO_ERROR) {
+		return err;
+	}
+#endif
+
 	/* Initialize CHIP server */
 #if CONFIG_CHIP_FACTORY_DATA
 	ReturnErrorOnFailure(mFactoryDataProvider.Init());

applications/matter_weather_station/src/app_task.cpp

@@ -217,6 +217,21 @@ CHIP_ERROR AppTask::Init()
 		return chip::System::MapErrorZephyr(ret);
 	}
 
+#ifdef CONFIG_CHIP_OTA_REQUESTOR
+	/* OTA image confirmation must be done before the factory data init. */
+	err = OtaConfirmNewImage();
+	if (err != CHIP_NO_ERROR) {
+		return err;
+	}
+#endif
+
+#ifdef CONFIG_MCUMGR_TRANSPORT_BT
+	/* Initialize DFU over SMP */
+	GetDFUOverSMP().Init();
+	GetDFUOverSMP().ConfirmNewImage();
+	GetDFUOverSMP().StartServer();
+#endif
+
 /* Get factory data */
 #ifdef CONFIG_CHIP_FACTORY_DATA
 	ReturnErrorOnFailure(mFactoryDataProvider.Init());
@@ -235,13 +250,6 @@ CHIP_ERROR AppTask::Init()
 	SetDeviceAttestationCredentialsProvider(Examples::GetExampleDACProvider());
 #endif
 
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT
-	/* Initialize DFU over SMP */
-	GetDFUOverSMP().Init();
-	GetDFUOverSMP().ConfirmNewImage();
-	GetDFUOverSMP().StartServer();
-#endif
-
 	/* Initialize timers */
 	k_timer_init(
 		&sFunctionTimer, [](k_timer *) { sAppTask.PostEvent(AppEvent{ AppEvent::FunctionTimer }); }, nullptr);

doc/nrf/releases_and_maturity/known_issues.rst

@@ -279,6 +279,56 @@ Matter
 
 The issues in this section are related to the :ref:`ug_matter` protocol.
 
+.. rst-class:: v2.4.1 v2-4-0 v2-3-0 v2-2-0
+
+KRKNWK-17535: The application core can crash on nRF5340 after the OTA firmware update finishes if the factory data module is enabled.
+  In the initialization method of the factory data module, the factory data partition and a part of the application image is restricted by Fprotect, which makes it impossible to confirm the new image in the Matter thread.
+  Instead, the confirmation must be performed before the factory data module is initialized.
+
+  **Affected platforms:** nRF5340
+
+  **Workaround:** Complete the following steps:
+
+  1. Manually cherry-pick and apply the commit with the fix to ``sdk-connectedhomeip`` (commit hash: ``TODO``).
+  #. Add the following lines to the :file:`samples/matter/common/src/ota_util.cpp`:
+
+     .. code-block::
+
+        #include <platform/CHIPDeviceLayer.h>
+        #include <zephyr/dfu/mcuboot.h>
+
+        CHIP_ERROR OtaConfirmNewImage()
+        {
+          CHIP_ERROR err = CHIP_NO_ERROR;
+          OTAImageProcessorImpl &imageProcessor = GetOTAImageProcessor();
+          if (imageProcessor.IsFirstImageRun()) {
+            CHIP_ERROR err = System::MapErrorZephyr(boot_write_img_confirmed());
+            if (CHIP_NO_ERROR == err) {
+              imageProcessor.SetImageConfirmed();
+            }
+          }
+          ChipLogError(SoftwareUpdate, "Failed to confirm firmware image, it will be reverted on the next boot");
+          return err;
+        }
+
+  #. Add the following line to the :file:`samples/matter/common/src/ota_util.h`:
+
+     .. code-block::
+
+        CHIP_ERROR OtaConfirmNewImage();
+
+  #. Add the following lines to the ``AppTask::Init()`` method in the :file:`app_task.cpp` file located in a sample directory before initialization of the factory data module (``mFactoryDataProvider.Init()``):
+
+     .. code-block::
+
+        #ifdef CONFIG_CHIP_OTA_REQUESTOR
+          /* OTA image confirmation must be done before the factory data init. */
+          err = OtaConfirmNewImage();
+          if (err != CHIP_NO_ERROR) {
+            return err;
+          }
+        #endif
+
 .. rst-class:: v2-4-0 v2-3-0
 
 KRKNWK-17151: Application core can crash on nRF5340 when there is a high load on Zephyr's main thread

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst

@@ -101,11 +101,16 @@ See `Bluetooth mesh samples`_ for the list of changes in the Bluetooth mesh samp
 Matter
 ------
 
-* Fixed an IPC crash on nRF5340 when Zephyr's main thread takes a long time.
 * Disabled OpenThread shell by default in Matter over Thread samples.
 * Enabled :kconfig:option:`CHIP_FACTORY_RESET_ERASE_NVS` Kconfig option by default, including for builds without factory data support.
   The firmware now erases all flash pages in the non-volatile storage during a factory reset, instead of just clearing Matter-related settings.
 
+* Fixed:
+
+  * An IPC crash on nRF5340 when Zephyr's main thread takes a long time.
+  * An application core crash on nRF5340 targets with the factory data module enabled.
+    The crash would happen after the OTA firmware update finishes and the image is confirmed.
+
 * Added:
 
   * Page about :ref:`ug_matter_device_optimizing_memory`.

samples/matter/common/src/ota_util.cpp

@@ -12,8 +12,12 @@
 #include <app/clusters/ota-requestor/DefaultOTARequestorDriver.h>
 #include <app/clusters/ota-requestor/DefaultOTARequestorStorage.h>
 #include <app/server/Server.h>
+#include <platform/CHIPDeviceLayer.h>
+#include <zephyr/dfu/mcuboot.h>
 #endif
 
+#include <lib/support/logging/CHIPLogging.h>
+
 using namespace chip;
 using namespace chip::DeviceLayer;
 
@@ -58,6 +62,21 @@ void InitBasicOTARequestor()
 	sOTARequestorDriver.Init(&sOTARequestor, &imageProcessor);
 	imageProcessor.TriggerFlashAction(ExternalFlashManager::Action::SLEEP);
 }
+
+CHIP_ERROR OtaConfirmNewImage()
+{
+	CHIP_ERROR err = CHIP_NO_ERROR;
+	OTAImageProcessorImpl &imageProcessor = GetOTAImageProcessor();
+	if (imageProcessor.IsFirstImageRun()) {
+		CHIP_ERROR err = System::MapErrorZephyr(boot_write_img_confirmed());
+		if (CHIP_NO_ERROR == err) {
+			imageProcessor.SetImageConfirmed();
+		}
+	}
+	ChipLogError(SoftwareUpdate, "Failed to confirm firmware image, it will be reverted on the next boot");
+	return err;
+}
+
 #endif
 
 ExternalFlashManager &GetFlashHandler()

samples/matter/common/src/ota_util.h

@@ -30,6 +30,16 @@ chip::DeviceLayer::OTAImageProcessorImpl &GetOTAImageProcessor();
  */
 void InitBasicOTARequestor();
 
+/**
+ * Check if the current image is the first boot the after OTA update and if so
+ * confirm it in MCUBoot.
+ *
+ * @return CHIP_NO_ERROR if the image has been confirmed, or it is not the first
+ * boot after the OTA update.
+ * Other CHIP_ERROR codes if the image could not be confirmed.
+ */
+CHIP_ERROR OtaConfirmNewImage();
+
 #endif /* CONFIG_CHIP_OTA_REQUESTOR */
 
 /**

samples/matter/light_bulb/src/app_task.cpp

@@ -177,12 +177,6 @@ CHIP_ERROR AppTask::Init()
 	/* Initialize trigger effect timer */
 	k_timer_init(&sTriggerEffectTimer, &AppTask::TriggerEffectTimerTimeoutCallback, nullptr);
 
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT
-	/* Initialize DFU over SMP */
-	GetDFUOverSMP().Init();
-	GetDFUOverSMP().ConfirmNewImage();
-#endif
-
 	/* Initialize lighting device (PWM) */
 	uint8_t minLightLevel = kDefaultMinLevel;
 	Clusters::LevelControl::Attributes::MinLevel::Get(kLightEndpointId, &minLightLevel);
@@ -196,6 +190,20 @@ CHIP_ERROR AppTask::Init()
 	}
 	mPWMDevice.SetCallbacks(ActionInitiated, ActionCompleted);
 
+#ifdef CONFIG_CHIP_OTA_REQUESTOR
+	/* OTA image confirmation must be done before the factory data init. */
+	err = OtaConfirmNewImage();
+	if (err != CHIP_NO_ERROR) {
+		return err;
+	}
+#endif
+
+#ifdef CONFIG_MCUMGR_TRANSPORT_BT
+	/* Initialize DFU over SMP */
+	GetDFUOverSMP().Init();
+	GetDFUOverSMP().ConfirmNewImage();
+#endif
+
 	/* Initialize CHIP server */
 #if CONFIG_CHIP_FACTORY_DATA
 	ReturnErrorOnFailure(mFactoryDataProvider.Init());

samples/matter/light_switch/src/app_task.cpp

@@ -170,6 +170,14 @@ CHIP_ERROR AppTask::Init()
 	k_timer_user_data_set(&sDimmerPressKeyTimer, this);
 	k_timer_user_data_set(&sFunctionTimer, this);
 
+#ifdef CONFIG_CHIP_OTA_REQUESTOR
+	/* OTA image confirmation must be done before the factory data init. */
+	err = OtaConfirmNewImage();
+	if (err != CHIP_NO_ERROR) {
+		return err;
+	}
+#endif
+
 #ifdef CONFIG_MCUMGR_TRANSPORT_BT
 	/* Initialize DFU over SMP */
 	GetDFUOverSMP().Init();

samples/matter/lock/src/app_task.cpp

@@ -196,12 +196,6 @@ CHIP_ERROR AppTask::Init()
 	k_timer_init(&sSwitchImagesTimer, &AppTask::SwitchImagesTimerTimeoutCallback, nullptr);
 #endif
 
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT
-	/* Initialize DFU over SMP */
-	GetDFUOverSMP().Init();
-	GetDFUOverSMP().ConfirmNewImage();
-#endif
-
 #ifdef CONFIG_CHIP_NUS
 	/* Initialize Nordic UART Service for Lock purposes */
 	if (!GetNUSService().Init(kLockNUSPriority, kAdvertisingIntervalMin, kAdvertisingIntervalMax)) {
@@ -217,6 +211,20 @@ CHIP_ERROR AppTask::Init()
 	/* Initialize lock manager */
 	BoltLockMgr().Init(LockStateChanged);
 
+#ifdef CONFIG_CHIP_OTA_REQUESTOR
+	/* OTA image confirmation must be done before the factory data init. */
+	err = OtaConfirmNewImage();
+	if (err != CHIP_NO_ERROR) {
+		return err;
+	}
+#endif
+
+#ifdef CONFIG_MCUMGR_TRANSPORT_BT
+	/* Initialize DFU over SMP */
+	GetDFUOverSMP().Init();
+	GetDFUOverSMP().ConfirmNewImage();
+#endif
+
 	/* Initialize CHIP server */
 #if CONFIG_CHIP_FACTORY_DATA
 	ReturnErrorOnFailure(mFactoryDataProvider.Init());

samples/matter/template/src/app_task.cpp

@@ -144,6 +144,14 @@ CHIP_ERROR AppTask::Init()
 	k_timer_init(&sFunctionTimer, &AppTask::FunctionTimerTimeoutCallback, nullptr);
 	k_timer_user_data_set(&sFunctionTimer, this);
 
+#ifdef CONFIG_CHIP_OTA_REQUESTOR
+	/* OTA image confirmation must be done before the factory data init. */
+	err = OtaConfirmNewImage();
+	if (err != CHIP_NO_ERROR) {
+		return err;
+	}
+#endif
+
 	/* Initialize CHIP server */
 #if CONFIG_CHIP_FACTORY_DATA
 	ReturnErrorOnFailure(mFactoryDataProvider.Init());

samples/matter/thermostat/src/app_task.cpp

@@ -162,6 +162,14 @@ CHIP_ERROR AppTask::Init()
 	k_timer_init(&sFunctionTimer, &AppTask::FunctionTimerTimeoutCallback, nullptr);
 	k_timer_user_data_set(&sFunctionTimer, this);
 
+#ifdef CONFIG_CHIP_OTA_REQUESTOR
+	/* OTA image confirmation must be done before the factory data init. */
+	err = OtaConfirmNewImage();
+	if (err != CHIP_NO_ERROR) {
+		return err;
+	}
+#endif
+
 #ifdef CONFIG_MCUMGR_TRANSPORT_BT
 	/* Initialize DFU over SMP */
 	GetDFUOverSMP().Init();

samples/matter/window_covering/src/app_task.cpp

@@ -134,6 +134,14 @@ CHIP_ERROR AppTask::Init()
 	k_timer_init(&sFunctionTimer, &AppTask::FunctionTimerTimeoutCallback, nullptr);
 	k_timer_user_data_set(&sFunctionTimer, this);
 
+#ifdef CONFIG_CHIP_OTA_REQUESTOR
+	/* OTA image confirmation must be done before the factory data init. */
+	err = OtaConfirmNewImage();
+	if (err != CHIP_NO_ERROR) {
+		return err;
+	}
+#endif
+
 #ifdef CONFIG_MCUMGR_TRANSPORT_BT
 	/* Initialize DFU over SMP */
 	GetDFUOverSMP().Init();

west.yml

@@ -147,7 +147,7 @@ manifest:
     - name: matter
       repo-path: sdk-connectedhomeip
       path: modules/lib/matter
-      revision: ad33cfbc151e0845017e1a978caded45cd7651eb
+      revision: eeb7280620fff1e16a75cfa41338186fd952c432
       submodules:
         - name: nlio
           path: third_party/nlio/repo