diff mbox series

[isar-cip-core] swupdate_2021.11: Backport raw handler unmount fix

Message ID 20240301121241.259677-1-tobias.schaffner@siemens.com (mailing list archive)
State Accepted
Headers show
Series [isar-cip-core] swupdate_2021.11: Backport raw handler unmount fix | expand

Commit Message

Tobias Schaffner March 1, 2024, 12:12 p.m. UTC
Swupdate 2021.11 has a bug that clears the boot partition if installing
a kernel file fails. On error the boot partition will stay mounted to a
directory in /tmp which will be recursively cleared.
This is critical if A/B partitioning with efibootguard is used as the ebg
environment will be destroyed making it impossible to do further updates.

Backport efc06332bb0bd622e6b542e0d3bd15135629d06a to fix this issue.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 ...ian-backport-raw-handler-unmount-fix.patch | 165 ++++++++++++++++++
 .../swupdate/swupdate_2021.11-1+debian-gbp.bb |   3 +-
 2 files changed, 167 insertions(+), 1 deletion(-)
 create mode 100644 recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch

Comments

Jan Kiszka March 5, 2024, 9:18 a.m. UTC | #1
On 01.03.24 13:12, Tobias Schaffner wrote:
> Swupdate 2021.11 has a bug that clears the boot partition if installing
> a kernel file fails. On error the boot partition will stay mounted to a
> directory in /tmp which will be recursively cleared.
> This is critical if A/B partitioning with efibootguard is used as the ebg
> environment will be destroyed making it impossible to do further updates.
> 
> Backport efc06332bb0bd622e6b542e0d3bd15135629d06a to fix this issue.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  ...ian-backport-raw-handler-unmount-fix.patch | 165 ++++++++++++++++++
>  .../swupdate/swupdate_2021.11-1+debian-gbp.bb |   3 +-
>  2 files changed, 167 insertions(+), 1 deletion(-)
>  create mode 100644 recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
> 
> diff --git a/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch b/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
> new file mode 100644
> index 0000000..fdea66c
> --- /dev/null
> +++ b/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
> @@ -0,0 +1,165 @@
> +From faae516d5c06dc13b953002fd91c0ba3e9bba7d8 Mon Sep 17 00:00:00 2001
> +From: Tobias Schaffner <tobias.schaffner@siemens.com>
> +Date: Thu, 15 Feb 2024 10:43:12 +0100
> +Subject: [PATCH] Backport raw handler unmount fix
> +
> +Swupdate 2021.11 has a bug that clears the boot partition if installing
> +a kernel file fails. On error the boot partition will stay mounted to a
> +directory in /tmp which will be recursively cleared.
> +This is critical if A/B partitioning with efibootguard is used as the ebg
> +environment will be destroyed making it impossible to do further updates.
> +
> +Backport efc06332bb0bd622e6b542e0d3bd15135629d06a to fix this issue.
> +
> +Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> +---
> + ...nmount-target-device-when-installing.patch | 128 ++++++++++++++++++
> + debian/patches/series                         |   1 +
> + 2 files changed, 129 insertions(+)
> + create mode 100644 debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
> +
> +diff --git a/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch b/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
> +new file mode 100644
> +index 00000000..f291a8bd
> +--- /dev/null
> ++++ b/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
> +@@ -0,0 +1,128 @@
> ++From d843a97aa56817c2c48dfb4f4b50315cdf753328 Mon Sep 17 00:00:00 2001
> ++From: Sava Jakovljev <savaj@meyersound.com>
> ++Date: Wed, 31 May 2023 23:44:22 +0200
> ++Subject: [PATCH] raw handler: Unmount target device when installing a file
> ++ fails
> ++
> ++Make sure to call 'swupdate_umount' in every scenario in
> ++'install_raw_file' function - this patch thus solves
> ++problems when an update fails on installing a file, and next update
> ++cannot be done because 'swupdate_mount' function fails with message
> ++"already mounted" because the  mount is still there from the previous
> ++attempt.
> ++
> ++Signed-off-by: Sava Jakovljev <savaj@meyersound.com>
> ++Signed-off-by: Stefano Babic <sbabic@denx.de>
> ++---
> ++ handlers/raw_handler.c | 65 +++++++++++++++++++++++++++++++-----------
> ++ 1 file changed, 49 insertions(+), 16 deletions(-)
> ++
> ++diff --git a/handlers/raw_handler.c b/handlers/raw_handler.c
> ++index 619a2f47..ac6a2030 100644
> ++--- a/handlers/raw_handler.c
> +++++ b/handlers/raw_handler.c
> ++@@ -206,8 +206,9 @@ static int install_raw_file(struct img_type *img,
> ++ 	void __attribute__ ((__unused__)) *data)
> ++ {
> ++ 	char path[255];
> ++-	int fdout;
> ++-	int ret = 0;
> +++	char tmp_path[255];
> +++	int fdout = -1;
> +++	int ret = -1;
> ++ 	int use_mount = (strlen(img->device) && strlen(img->filesystem)) ? 1 : 0;
> ++ 	char* DATADST_DIR = alloca(strlen(get_tmpdir())+strlen(DATADST_DIR_SUFFIX)+1);
> ++ 	sprintf(DATADST_DIR, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX);
> ++@@ -228,7 +229,7 @@ static int install_raw_file(struct img_type *img,
> ++ 		if (snprintf(path, sizeof(path), "%s%s",
> ++ 					 DATADST_DIR, img->path) >= (int)sizeof(path)) {
> ++ 			ERROR("Path too long: %s%s", DATADST_DIR, img->path);
> ++-			return -1;
> +++			goto cleanup;
> ++ 		}
> ++ 	} else {
> ++ 		if (snprintf(path, sizeof(path), "%s", img->path) >= (int)sizeof(path)) {
> ++@@ -237,35 +238,67 @@ static int install_raw_file(struct img_type *img,
> ++ 		}
> ++ 	}
> ++ 
> ++-	TRACE("Installing file %s on %s",
> ++-		img->fname, path);
> +++	if (strtobool(dict_get_value(&img->properties, "atomic-install"))) {
> +++		if (snprintf(tmp_path, sizeof(tmp_path), "%s.tmp", path) >= (int)sizeof(tmp_path)) {
> +++			ERROR("Temp path too long: %s.tmp", img->path);
> +++			ret = -1;
> +++			goto cleanup;
> +++		}
> +++	}
> +++	else {
> +++		snprintf(tmp_path, sizeof(tmp_path), "%s", path);
> +++	}
> +++	TRACE("Installing file %s on %s", img->fname, tmp_path);
> ++ 
> ++ 	if (strtobool(dict_get_value(&img->properties, "create-destination"))) {
> ++ 		TRACE("Creating path %s", path);
> ++-		fdout = mkpath(dirname(strdupa(path)), 0755);
> ++-		if (fdout < 0) {
> +++		ret = mkpath(dirname(strdupa(path)), 0755);
> +++		if (ret < 0) {
> ++ 			ERROR("I cannot create path %s: %s", path, strerror(errno));
> ++-			return -1;
> +++			goto cleanup;
> ++ 		}
> ++ 	}
> ++ 
> ++-	fdout = openfileoutput(path);
> ++-	if (fdout < 0)
> ++-		return fdout;
> +++	fdout = openfileoutput(tmp_path);
> +++	if (fdout < 0) {
> +++		ret = -1;
> +++		goto cleanup;
> +++	}
> ++ 	if (!img_check_free_space(img, fdout)) {
> ++-		return -ENOSPC;
> +++		ret = -ENOSPC;
> +++		goto cleanup;
> ++ 	}
> ++ 
> ++ 	ret = copyimage(&fdout, img, NULL);
> ++-	if (ret< 0) {
> +++	if (ret < 0) {
> ++ 		ERROR("Error copying extracted file");
> +++		goto cleanup;
> ++ 	}
> ++-	close(fdout);
> ++ 
> ++-	if (use_mount) {
> ++-		swupdate_umount(DATADST_DIR);
> +++	if (fsync(fdout)) {
> +++		ERROR("Error writing %s to disk: %s", tmp_path, strerror(errno));
> +++		ret = -1;
> +++		goto cleanup;
> +++	}
> +++
> +++	if (strtobool(dict_get_value(&img->properties, "atomic-install"))) {
> +++		TRACE("Renaming file %s to %s", tmp_path, path);
> +++		if (rename(tmp_path, path)) {
> +++			ERROR("Error renaming %s to %s: %s", tmp_path, path, strerror(errno));
> +++			ret = -1;
> +++			goto cleanup;
> +++		}
> ++ 	}
> ++ 
> +++	ret = 0;
> +++
> +++cleanup:
> +++	if (fdout > 0)
> +++		close(fdout);
> +++
> +++	if (use_mount)
> +++		swupdate_umount(DATADST_DIR);
> +++
> ++ 	return ret;
> ++ }
> ++ 
> ++-- 
> ++2.34.1
> ++
> +diff --git a/debian/patches/series b/debian/patches/series
> +index 98628a77..21a82863 100644
> +--- a/debian/patches/series
> ++++ b/debian/patches/series
> +@@ -1,2 +1,3 @@
> + use-gcc-compiler.diff
> + 0001-bootloader-EBG-fix-do_env_get-for-anything-but-globa.patch
> ++0002-raw-handler-Unmount-target-device-when-installing.patch
> +-- 
> +2.34.1
> +
> diff --git a/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb b/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
> index 2384f41..5671cfa 100644
> --- a/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
> +++ b/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
> @@ -25,7 +25,8 @@ SRC_URI += "file://0001-debian-Remove-SWUpdate-USB-service-and-Udev-rules.patch
>              file://0002-debian-rules-Add-Embedded-Lua-handler-option.patch \
>              file://0003-debian-rules-Add-option-to-disable-fs-creation.patch \
>              file://0004-debian-rules-Add-option-to-disable-webserver.patch \
> -            file://0005-debian-Add-patch-to-fix-bootloader_env_get-for-EBG.patch"
> +            file://0005-debian-Add-patch-to-fix-bootloader_env_get-for-EBG.patch \
> +            file://0006-debian-backport-raw-handler-unmount-fix.patch"
>  
>  # If the luahandler shall be embedded into the swupdate binary
>  # include the following lines.

Looks good to me. Quirin, any remarks?

Jan
Gylstorff Quirin March 5, 2024, 10:55 a.m. UTC | #2
On 3/5/24 10:18 AM, Jan Kiszka wrote:
> On 01.03.24 13:12, Tobias Schaffner wrote:
>> Swupdate 2021.11 has a bug that clears the boot partition if installing
>> a kernel file fails. On error the boot partition will stay mounted to a
>> directory in /tmp which will be recursively cleared.
>> This is critical if A/B partitioning with efibootguard is used as the ebg
>> environment will be destroyed making it impossible to do further updates.
>>
>> Backport efc06332bb0bd622e6b542e0d3bd15135629d06a to fix this issue.
>>
>> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
>> ---
>>   ...ian-backport-raw-handler-unmount-fix.patch | 165 ++++++++++++++++++
>>   .../swupdate/swupdate_2021.11-1+debian-gbp.bb |   3 +-
>>   2 files changed, 167 insertions(+), 1 deletion(-)
>>   create mode 100644 recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
>>
>> diff --git a/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch b/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
>> new file mode 100644
>> index 0000000..fdea66c
>> --- /dev/null
>> +++ b/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
>> @@ -0,0 +1,165 @@
>> +From faae516d5c06dc13b953002fd91c0ba3e9bba7d8 Mon Sep 17 00:00:00 2001
>> +From: Tobias Schaffner <tobias.schaffner@siemens.com>
>> +Date: Thu, 15 Feb 2024 10:43:12 +0100
>> +Subject: [PATCH] Backport raw handler unmount fix
>> +
>> +Swupdate 2021.11 has a bug that clears the boot partition if installing
>> +a kernel file fails. On error the boot partition will stay mounted to a
>> +directory in /tmp which will be recursively cleared.
>> +This is critical if A/B partitioning with efibootguard is used as the ebg
>> +environment will be destroyed making it impossible to do further updates.
>> +
>> +Backport efc06332bb0bd622e6b542e0d3bd15135629d06a to fix this issue.
>> +
>> +Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
>> +---
>> + ...nmount-target-device-when-installing.patch | 128 ++++++++++++++++++
>> + debian/patches/series                         |   1 +
>> + 2 files changed, 129 insertions(+)
>> + create mode 100644 debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
>> +
>> +diff --git a/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch b/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
>> +new file mode 100644
>> +index 00000000..f291a8bd
>> +--- /dev/null
>> ++++ b/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
>> +@@ -0,0 +1,128 @@
>> ++From d843a97aa56817c2c48dfb4f4b50315cdf753328 Mon Sep 17 00:00:00 2001
>> ++From: Sava Jakovljev <savaj@meyersound.com>
>> ++Date: Wed, 31 May 2023 23:44:22 +0200
>> ++Subject: [PATCH] raw handler: Unmount target device when installing a file
>> ++ fails
>> ++
>> ++Make sure to call 'swupdate_umount' in every scenario in
>> ++'install_raw_file' function - this patch thus solves
>> ++problems when an update fails on installing a file, and next update
>> ++cannot be done because 'swupdate_mount' function fails with message
>> ++"already mounted" because the  mount is still there from the previous
>> ++attempt.
>> ++
>> ++Signed-off-by: Sava Jakovljev <savaj@meyersound.com>
>> ++Signed-off-by: Stefano Babic <sbabic@denx.de>
>> ++---
>> ++ handlers/raw_handler.c | 65 +++++++++++++++++++++++++++++++-----------
>> ++ 1 file changed, 49 insertions(+), 16 deletions(-)
>> ++
>> ++diff --git a/handlers/raw_handler.c b/handlers/raw_handler.c
>> ++index 619a2f47..ac6a2030 100644
>> ++--- a/handlers/raw_handler.c
>> +++++ b/handlers/raw_handler.c
>> ++@@ -206,8 +206,9 @@ static int install_raw_file(struct img_type *img,
>> ++ 	void __attribute__ ((__unused__)) *data)
>> ++ {
>> ++ 	char path[255];
>> ++-	int fdout;
>> ++-	int ret = 0;
>> +++	char tmp_path[255];
>> +++	int fdout = -1;
>> +++	int ret = -1;
>> ++ 	int use_mount = (strlen(img->device) && strlen(img->filesystem)) ? 1 : 0;
>> ++ 	char* DATADST_DIR = alloca(strlen(get_tmpdir())+strlen(DATADST_DIR_SUFFIX)+1);
>> ++ 	sprintf(DATADST_DIR, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX);
>> ++@@ -228,7 +229,7 @@ static int install_raw_file(struct img_type *img,
>> ++ 		if (snprintf(path, sizeof(path), "%s%s",
>> ++ 					 DATADST_DIR, img->path) >= (int)sizeof(path)) {
>> ++ 			ERROR("Path too long: %s%s", DATADST_DIR, img->path);
>> ++-			return -1;
>> +++			goto cleanup;
>> ++ 		}
>> ++ 	} else {
>> ++ 		if (snprintf(path, sizeof(path), "%s", img->path) >= (int)sizeof(path)) {
>> ++@@ -237,35 +238,67 @@ static int install_raw_file(struct img_type *img,
>> ++ 		}
>> ++ 	}
>> ++
>> ++-	TRACE("Installing file %s on %s",
>> ++-		img->fname, path);
>> +++	if (strtobool(dict_get_value(&img->properties, "atomic-install"))) {
>> +++		if (snprintf(tmp_path, sizeof(tmp_path), "%s.tmp", path) >= (int)sizeof(tmp_path)) {
>> +++			ERROR("Temp path too long: %s.tmp", img->path);
>> +++			ret = -1;
>> +++			goto cleanup;
>> +++		}
>> +++	}
>> +++	else {
>> +++		snprintf(tmp_path, sizeof(tmp_path), "%s", path);
>> +++	}
>> +++	TRACE("Installing file %s on %s", img->fname, tmp_path);
>> ++
>> ++ 	if (strtobool(dict_get_value(&img->properties, "create-destination"))) {
>> ++ 		TRACE("Creating path %s", path);
>> ++-		fdout = mkpath(dirname(strdupa(path)), 0755);
>> ++-		if (fdout < 0) {
>> +++		ret = mkpath(dirname(strdupa(path)), 0755);
>> +++		if (ret < 0) {
>> ++ 			ERROR("I cannot create path %s: %s", path, strerror(errno));
>> ++-			return -1;
>> +++			goto cleanup;
>> ++ 		}
>> ++ 	}
>> ++
>> ++-	fdout = openfileoutput(path);
>> ++-	if (fdout < 0)
>> ++-		return fdout;
>> +++	fdout = openfileoutput(tmp_path);
>> +++	if (fdout < 0) {
>> +++		ret = -1;
>> +++		goto cleanup;
>> +++	}
>> ++ 	if (!img_check_free_space(img, fdout)) {
>> ++-		return -ENOSPC;
>> +++		ret = -ENOSPC;
>> +++		goto cleanup;
>> ++ 	}
>> ++
>> ++ 	ret = copyimage(&fdout, img, NULL);
>> ++-	if (ret< 0) {
>> +++	if (ret < 0) {
>> ++ 		ERROR("Error copying extracted file");
>> +++		goto cleanup;
>> ++ 	}
>> ++-	close(fdout);
>> ++
>> ++-	if (use_mount) {
>> ++-		swupdate_umount(DATADST_DIR);
>> +++	if (fsync(fdout)) {
>> +++		ERROR("Error writing %s to disk: %s", tmp_path, strerror(errno));
>> +++		ret = -1;
>> +++		goto cleanup;
>> +++	}
>> +++
>> +++	if (strtobool(dict_get_value(&img->properties, "atomic-install"))) {
>> +++		TRACE("Renaming file %s to %s", tmp_path, path);
>> +++		if (rename(tmp_path, path)) {
>> +++			ERROR("Error renaming %s to %s: %s", tmp_path, path, strerror(errno));
>> +++			ret = -1;
>> +++			goto cleanup;
>> +++		}
>> ++ 	}
>> ++
>> +++	ret = 0;
>> +++
>> +++cleanup:
>> +++	if (fdout > 0)
>> +++		close(fdout);
>> +++
>> +++	if (use_mount)
>> +++		swupdate_umount(DATADST_DIR);
>> +++
>> ++ 	return ret;
>> ++ }
>> ++
>> ++--
>> ++2.34.1
>> ++
>> +diff --git a/debian/patches/series b/debian/patches/series
>> +index 98628a77..21a82863 100644
>> +--- a/debian/patches/series
>> ++++ b/debian/patches/series
>> +@@ -1,2 +1,3 @@
>> + use-gcc-compiler.diff
>> + 0001-bootloader-EBG-fix-do_env_get-for-anything-but-globa.patch
>> ++0002-raw-handler-Unmount-target-device-when-installing.patch
>> +--
>> +2.34.1
>> +
>> diff --git a/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb b/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
>> index 2384f41..5671cfa 100644
>> --- a/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
>> +++ b/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
>> @@ -25,7 +25,8 @@ SRC_URI += "file://0001-debian-Remove-SWUpdate-USB-service-and-Udev-rules.patch
>>               file://0002-debian-rules-Add-Embedded-Lua-handler-option.patch \
>>               file://0003-debian-rules-Add-option-to-disable-fs-creation.patch \
>>               file://0004-debian-rules-Add-option-to-disable-webserver.patch \
>> -            file://0005-debian-Add-patch-to-fix-bootloader_env_get-for-EBG.patch"
>> +            file://0005-debian-Add-patch-to-fix-bootloader_env_get-for-EBG.patch \
>> +            file://0006-debian-backport-raw-handler-unmount-fix.patch"
>>   
>>   # If the luahandler shall be embedded into the swupdate binary
>>   # include the following lines.
> 
> Looks good to me. Quirin, any remarks?

Looks good to me.

Quirin
> 
> Jan
>
Jan Kiszka March 5, 2024, 2:13 p.m. UTC | #3
On 05.03.24 11:55, Gylstorff Quirin wrote:
> 
> 
> On 3/5/24 10:18 AM, Jan Kiszka wrote:
>> On 01.03.24 13:12, Tobias Schaffner wrote:
>>> Swupdate 2021.11 has a bug that clears the boot partition if installing
>>> a kernel file fails. On error the boot partition will stay mounted to a
>>> directory in /tmp which will be recursively cleared.
>>> This is critical if A/B partitioning with efibootguard is used as the
>>> ebg
>>> environment will be destroyed making it impossible to do further
>>> updates.
>>>
>>> Backport efc06332bb0bd622e6b542e0d3bd15135629d06a to fix this issue.
>>>
>>> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
>>> ---
>>>   ...ian-backport-raw-handler-unmount-fix.patch | 165 ++++++++++++++++++
>>>   .../swupdate/swupdate_2021.11-1+debian-gbp.bb |   3 +-
>>>   2 files changed, 167 insertions(+), 1 deletion(-)
>>>   create mode 100644
>>> recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
>>>
>>> diff --git
>>> a/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch b/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
>>> new file mode 100644
>>> index 0000000..fdea66c
>>> --- /dev/null
>>> +++
>>> b/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
>>> @@ -0,0 +1,165 @@
>>> +From faae516d5c06dc13b953002fd91c0ba3e9bba7d8 Mon Sep 17 00:00:00 2001
>>> +From: Tobias Schaffner <tobias.schaffner@siemens.com>
>>> +Date: Thu, 15 Feb 2024 10:43:12 +0100
>>> +Subject: [PATCH] Backport raw handler unmount fix
>>> +
>>> +Swupdate 2021.11 has a bug that clears the boot partition if installing
>>> +a kernel file fails. On error the boot partition will stay mounted to a
>>> +directory in /tmp which will be recursively cleared.
>>> +This is critical if A/B partitioning with efibootguard is used as
>>> the ebg
>>> +environment will be destroyed making it impossible to do further
>>> updates.
>>> +
>>> +Backport efc06332bb0bd622e6b542e0d3bd15135629d06a to fix this issue.
>>> +
>>> +Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
>>> +---
>>> + ...nmount-target-device-when-installing.patch | 128 ++++++++++++++++++
>>> + debian/patches/series                         |   1 +
>>> + 2 files changed, 129 insertions(+)
>>> + create mode 100644
>>> debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
>>> +
>>> +diff --git
>>> a/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch b/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
>>> +new file mode 100644
>>> +index 00000000..f291a8bd
>>> +--- /dev/null
>>> ++++
>>> b/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
>>> +@@ -0,0 +1,128 @@
>>> ++From d843a97aa56817c2c48dfb4f4b50315cdf753328 Mon Sep 17 00:00:00 2001
>>> ++From: Sava Jakovljev <savaj@meyersound.com>
>>> ++Date: Wed, 31 May 2023 23:44:22 +0200
>>> ++Subject: [PATCH] raw handler: Unmount target device when installing
>>> a file
>>> ++ fails
>>> ++
>>> ++Make sure to call 'swupdate_umount' in every scenario in
>>> ++'install_raw_file' function - this patch thus solves
>>> ++problems when an update fails on installing a file, and next update
>>> ++cannot be done because 'swupdate_mount' function fails with message
>>> ++"already mounted" because the  mount is still there from the previous
>>> ++attempt.
>>> ++
>>> ++Signed-off-by: Sava Jakovljev <savaj@meyersound.com>
>>> ++Signed-off-by: Stefano Babic <sbabic@denx.de>
>>> ++---
>>> ++ handlers/raw_handler.c | 65
>>> +++++++++++++++++++++++++++++++-----------
>>> ++ 1 file changed, 49 insertions(+), 16 deletions(-)
>>> ++
>>> ++diff --git a/handlers/raw_handler.c b/handlers/raw_handler.c
>>> ++index 619a2f47..ac6a2030 100644
>>> ++--- a/handlers/raw_handler.c
>>> +++++ b/handlers/raw_handler.c
>>> ++@@ -206,8 +206,9 @@ static int install_raw_file(struct img_type *img,
>>> ++     void __attribute__ ((__unused__)) *data)
>>> ++ {
>>> ++     char path[255];
>>> ++-    int fdout;
>>> ++-    int ret = 0;
>>> +++    char tmp_path[255];
>>> +++    int fdout = -1;
>>> +++    int ret = -1;
>>> ++     int use_mount = (strlen(img->device) &&
>>> strlen(img->filesystem)) ? 1 : 0;
>>> ++     char* DATADST_DIR =
>>> alloca(strlen(get_tmpdir())+strlen(DATADST_DIR_SUFFIX)+1);
>>> ++     sprintf(DATADST_DIR, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX);
>>> ++@@ -228,7 +229,7 @@ static int install_raw_file(struct img_type *img,
>>> ++         if (snprintf(path, sizeof(path), "%s%s",
>>> ++                      DATADST_DIR, img->path) >= (int)sizeof(path)) {
>>> ++             ERROR("Path too long: %s%s", DATADST_DIR, img->path);
>>> ++-            return -1;
>>> +++            goto cleanup;
>>> ++         }
>>> ++     } else {
>>> ++         if (snprintf(path, sizeof(path), "%s", img->path) >=
>>> (int)sizeof(path)) {
>>> ++@@ -237,35 +238,67 @@ static int install_raw_file(struct img_type
>>> *img,
>>> ++         }
>>> ++     }
>>> ++
>>> ++-    TRACE("Installing file %s on %s",
>>> ++-        img->fname, path);
>>> +++    if (strtobool(dict_get_value(&img->properties,
>>> "atomic-install"))) {
>>> +++        if (snprintf(tmp_path, sizeof(tmp_path), "%s.tmp", path)
>>> >= (int)sizeof(tmp_path)) {
>>> +++            ERROR("Temp path too long: %s.tmp", img->path);
>>> +++            ret = -1;
>>> +++            goto cleanup;
>>> +++        }
>>> +++    }
>>> +++    else {
>>> +++        snprintf(tmp_path, sizeof(tmp_path), "%s", path);
>>> +++    }
>>> +++    TRACE("Installing file %s on %s", img->fname, tmp_path);
>>> ++
>>> ++     if (strtobool(dict_get_value(&img->properties,
>>> "create-destination"))) {
>>> ++         TRACE("Creating path %s", path);
>>> ++-        fdout = mkpath(dirname(strdupa(path)), 0755);
>>> ++-        if (fdout < 0) {
>>> +++        ret = mkpath(dirname(strdupa(path)), 0755);
>>> +++        if (ret < 0) {
>>> ++             ERROR("I cannot create path %s: %s", path,
>>> strerror(errno));
>>> ++-            return -1;
>>> +++            goto cleanup;
>>> ++         }
>>> ++     }
>>> ++
>>> ++-    fdout = openfileoutput(path);
>>> ++-    if (fdout < 0)
>>> ++-        return fdout;
>>> +++    fdout = openfileoutput(tmp_path);
>>> +++    if (fdout < 0) {
>>> +++        ret = -1;
>>> +++        goto cleanup;
>>> +++    }
>>> ++     if (!img_check_free_space(img, fdout)) {
>>> ++-        return -ENOSPC;
>>> +++        ret = -ENOSPC;
>>> +++        goto cleanup;
>>> ++     }
>>> ++
>>> ++     ret = copyimage(&fdout, img, NULL);
>>> ++-    if (ret< 0) {
>>> +++    if (ret < 0) {
>>> ++         ERROR("Error copying extracted file");
>>> +++        goto cleanup;
>>> ++     }
>>> ++-    close(fdout);
>>> ++
>>> ++-    if (use_mount) {
>>> ++-        swupdate_umount(DATADST_DIR);
>>> +++    if (fsync(fdout)) {
>>> +++        ERROR("Error writing %s to disk: %s", tmp_path,
>>> strerror(errno));
>>> +++        ret = -1;
>>> +++        goto cleanup;
>>> +++    }
>>> +++
>>> +++    if (strtobool(dict_get_value(&img->properties,
>>> "atomic-install"))) {
>>> +++        TRACE("Renaming file %s to %s", tmp_path, path);
>>> +++        if (rename(tmp_path, path)) {
>>> +++            ERROR("Error renaming %s to %s: %s", tmp_path, path,
>>> strerror(errno));
>>> +++            ret = -1;
>>> +++            goto cleanup;
>>> +++        }
>>> ++     }
>>> ++
>>> +++    ret = 0;
>>> +++
>>> +++cleanup:
>>> +++    if (fdout > 0)
>>> +++        close(fdout);
>>> +++
>>> +++    if (use_mount)
>>> +++        swupdate_umount(DATADST_DIR);
>>> +++
>>> ++     return ret;
>>> ++ }
>>> ++
>>> ++--
>>> ++2.34.1
>>> ++
>>> +diff --git a/debian/patches/series b/debian/patches/series
>>> +index 98628a77..21a82863 100644
>>> +--- a/debian/patches/series
>>> ++++ b/debian/patches/series
>>> +@@ -1,2 +1,3 @@
>>> + use-gcc-compiler.diff
>>> + 0001-bootloader-EBG-fix-do_env_get-for-anything-but-globa.patch
>>> ++0002-raw-handler-Unmount-target-device-when-installing.patch
>>> +--
>>> +2.34.1
>>> +
>>> diff --git a/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
>>> b/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
>>> index 2384f41..5671cfa 100644
>>> --- a/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
>>> +++ b/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
>>> @@ -25,7 +25,8 @@ SRC_URI +=
>>> "file://0001-debian-Remove-SWUpdate-USB-service-and-Udev-rules.patch
>>>              
>>> file://0002-debian-rules-Add-Embedded-Lua-handler-option.patch \
>>>              
>>> file://0003-debian-rules-Add-option-to-disable-fs-creation.patch \
>>>              
>>> file://0004-debian-rules-Add-option-to-disable-webserver.patch \
>>> -           
>>> file://0005-debian-Add-patch-to-fix-bootloader_env_get-for-EBG.patch"
>>> +           
>>> file://0005-debian-Add-patch-to-fix-bootloader_env_get-for-EBG.patch \
>>> +            file://0006-debian-backport-raw-handler-unmount-fix.patch"
>>>     # If the luahandler shall be embedded into the swupdate binary
>>>   # include the following lines.
>>
>> Looks good to me. Quirin, any remarks?
> 
> Looks good to me.
> 

Applied to next, thanks.

Jan
diff mbox series

Patch

diff --git a/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch b/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
new file mode 100644
index 0000000..fdea66c
--- /dev/null
+++ b/recipes-core/swupdate/files/2021.11/0006-debian-backport-raw-handler-unmount-fix.patch
@@ -0,0 +1,165 @@ 
+From faae516d5c06dc13b953002fd91c0ba3e9bba7d8 Mon Sep 17 00:00:00 2001
+From: Tobias Schaffner <tobias.schaffner@siemens.com>
+Date: Thu, 15 Feb 2024 10:43:12 +0100
+Subject: [PATCH] Backport raw handler unmount fix
+
+Swupdate 2021.11 has a bug that clears the boot partition if installing
+a kernel file fails. On error the boot partition will stay mounted to a
+directory in /tmp which will be recursively cleared.
+This is critical if A/B partitioning with efibootguard is used as the ebg
+environment will be destroyed making it impossible to do further updates.
+
+Backport efc06332bb0bd622e6b542e0d3bd15135629d06a to fix this issue.
+
+Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
+---
+ ...nmount-target-device-when-installing.patch | 128 ++++++++++++++++++
+ debian/patches/series                         |   1 +
+ 2 files changed, 129 insertions(+)
+ create mode 100644 debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
+
+diff --git a/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch b/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
+new file mode 100644
+index 00000000..f291a8bd
+--- /dev/null
++++ b/debian/patches/0002-raw-handler-Unmount-target-device-when-installing.patch
+@@ -0,0 +1,128 @@
++From d843a97aa56817c2c48dfb4f4b50315cdf753328 Mon Sep 17 00:00:00 2001
++From: Sava Jakovljev <savaj@meyersound.com>
++Date: Wed, 31 May 2023 23:44:22 +0200
++Subject: [PATCH] raw handler: Unmount target device when installing a file
++ fails
++
++Make sure to call 'swupdate_umount' in every scenario in
++'install_raw_file' function - this patch thus solves
++problems when an update fails on installing a file, and next update
++cannot be done because 'swupdate_mount' function fails with message
++"already mounted" because the  mount is still there from the previous
++attempt.
++
++Signed-off-by: Sava Jakovljev <savaj@meyersound.com>
++Signed-off-by: Stefano Babic <sbabic@denx.de>
++---
++ handlers/raw_handler.c | 65 +++++++++++++++++++++++++++++++-----------
++ 1 file changed, 49 insertions(+), 16 deletions(-)
++
++diff --git a/handlers/raw_handler.c b/handlers/raw_handler.c
++index 619a2f47..ac6a2030 100644
++--- a/handlers/raw_handler.c
+++++ b/handlers/raw_handler.c
++@@ -206,8 +206,9 @@ static int install_raw_file(struct img_type *img,
++ 	void __attribute__ ((__unused__)) *data)
++ {
++ 	char path[255];
++-	int fdout;
++-	int ret = 0;
+++	char tmp_path[255];
+++	int fdout = -1;
+++	int ret = -1;
++ 	int use_mount = (strlen(img->device) && strlen(img->filesystem)) ? 1 : 0;
++ 	char* DATADST_DIR = alloca(strlen(get_tmpdir())+strlen(DATADST_DIR_SUFFIX)+1);
++ 	sprintf(DATADST_DIR, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX);
++@@ -228,7 +229,7 @@ static int install_raw_file(struct img_type *img,
++ 		if (snprintf(path, sizeof(path), "%s%s",
++ 					 DATADST_DIR, img->path) >= (int)sizeof(path)) {
++ 			ERROR("Path too long: %s%s", DATADST_DIR, img->path);
++-			return -1;
+++			goto cleanup;
++ 		}
++ 	} else {
++ 		if (snprintf(path, sizeof(path), "%s", img->path) >= (int)sizeof(path)) {
++@@ -237,35 +238,67 @@ static int install_raw_file(struct img_type *img,
++ 		}
++ 	}
++ 
++-	TRACE("Installing file %s on %s",
++-		img->fname, path);
+++	if (strtobool(dict_get_value(&img->properties, "atomic-install"))) {
+++		if (snprintf(tmp_path, sizeof(tmp_path), "%s.tmp", path) >= (int)sizeof(tmp_path)) {
+++			ERROR("Temp path too long: %s.tmp", img->path);
+++			ret = -1;
+++			goto cleanup;
+++		}
+++	}
+++	else {
+++		snprintf(tmp_path, sizeof(tmp_path), "%s", path);
+++	}
+++	TRACE("Installing file %s on %s", img->fname, tmp_path);
++ 
++ 	if (strtobool(dict_get_value(&img->properties, "create-destination"))) {
++ 		TRACE("Creating path %s", path);
++-		fdout = mkpath(dirname(strdupa(path)), 0755);
++-		if (fdout < 0) {
+++		ret = mkpath(dirname(strdupa(path)), 0755);
+++		if (ret < 0) {
++ 			ERROR("I cannot create path %s: %s", path, strerror(errno));
++-			return -1;
+++			goto cleanup;
++ 		}
++ 	}
++ 
++-	fdout = openfileoutput(path);
++-	if (fdout < 0)
++-		return fdout;
+++	fdout = openfileoutput(tmp_path);
+++	if (fdout < 0) {
+++		ret = -1;
+++		goto cleanup;
+++	}
++ 	if (!img_check_free_space(img, fdout)) {
++-		return -ENOSPC;
+++		ret = -ENOSPC;
+++		goto cleanup;
++ 	}
++ 
++ 	ret = copyimage(&fdout, img, NULL);
++-	if (ret< 0) {
+++	if (ret < 0) {
++ 		ERROR("Error copying extracted file");
+++		goto cleanup;
++ 	}
++-	close(fdout);
++ 
++-	if (use_mount) {
++-		swupdate_umount(DATADST_DIR);
+++	if (fsync(fdout)) {
+++		ERROR("Error writing %s to disk: %s", tmp_path, strerror(errno));
+++		ret = -1;
+++		goto cleanup;
+++	}
+++
+++	if (strtobool(dict_get_value(&img->properties, "atomic-install"))) {
+++		TRACE("Renaming file %s to %s", tmp_path, path);
+++		if (rename(tmp_path, path)) {
+++			ERROR("Error renaming %s to %s: %s", tmp_path, path, strerror(errno));
+++			ret = -1;
+++			goto cleanup;
+++		}
++ 	}
++ 
+++	ret = 0;
+++
+++cleanup:
+++	if (fdout > 0)
+++		close(fdout);
+++
+++	if (use_mount)
+++		swupdate_umount(DATADST_DIR);
+++
++ 	return ret;
++ }
++ 
++-- 
++2.34.1
++
+diff --git a/debian/patches/series b/debian/patches/series
+index 98628a77..21a82863 100644
+--- a/debian/patches/series
++++ b/debian/patches/series
+@@ -1,2 +1,3 @@
+ use-gcc-compiler.diff
+ 0001-bootloader-EBG-fix-do_env_get-for-anything-but-globa.patch
++0002-raw-handler-Unmount-target-device-when-installing.patch
+-- 
+2.34.1
+
diff --git a/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb b/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
index 2384f41..5671cfa 100644
--- a/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
+++ b/recipes-core/swupdate/swupdate_2021.11-1+debian-gbp.bb
@@ -25,7 +25,8 @@  SRC_URI += "file://0001-debian-Remove-SWUpdate-USB-service-and-Udev-rules.patch
             file://0002-debian-rules-Add-Embedded-Lua-handler-option.patch \
             file://0003-debian-rules-Add-option-to-disable-fs-creation.patch \
             file://0004-debian-rules-Add-option-to-disable-webserver.patch \
-            file://0005-debian-Add-patch-to-fix-bootloader_env_get-for-EBG.patch"
+            file://0005-debian-Add-patch-to-fix-bootloader_env_get-for-EBG.patch \
+            file://0006-debian-backport-raw-handler-unmount-fix.patch"
 
 # If the luahandler shall be embedded into the swupdate binary
 # include the following lines.