[OpenWrt-Devel] [PATCH procd] system: reject sysupgrade of broken firmware images

[Reiner Karlsberg]

This needs to be handled very carefully, not to break
actual usage of -F.
I had to use -F couple of times, usually when downgrading
installed firmware, but with change of name over time.
Typical example: Change of image name for the zbt-we826.
Never any problem with usage of -F, though.

But I had several problems with non-completion of
valid sysupgrade, which even left the system in inconsistent state,
as the "-f keep.tar.gz" was applied, but not the new image itself.
Or (silently ?) no sysupgrade done, because of mounted block device
or active swap file on block device, or active wifi (?).
Which was a PITA on remote installations.

Question: How are sysupgrade-errors reported/to be handled, as usually also a failed sysupgrade
triggered a reboot ?
documentation very unclear here, as it looks like, behaviour in case of errro changed during versions of openwrt.

Best would be "atomic sysupgrade", with standard error-code (+1) on exit instead of expected reboot after success.

I appreciate the new work on sysupgrade, but I am a bit afraid of regressions.



Am 01.09.2019 um 01:14 schrieb Karl Palsson:
> 
> What's the point of "force" if it doesn't force? Are we going to
> add a second -F to "really force" ? Or is it going to be "oh, -F
> failed for some lame reason, so I'll use mtd write, and still
> complain anyway"
> 
> Cheers,
> Karl P
> 
> Rafał Miłecki  <zajec5 at gmail.com> wrote:
>> From: Rafał Miłecki <rafal at milecki.pl>
>>
>> This uses recently added "validate_firmware_image" to validate
>> passed firmware. If it happens to be invalid and marked as
>> impossible to force then sysupgrade simply exits with an error.
>>
>> This change is needed to avoid bricking devices with some
>> totally broken images.
>>
>> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
>> ---
>> This patch depends on the:
>> [PATCH procd] system: add "validate_firmware_image" ubus method
>> ---
>>   system.c | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/system.c b/system.c
>> index 35d5a23..7f49758 100644
>> --- a/system.c
>> +++ b/system.c
>> @@ -507,7 +507,18 @@ static int sysupgrade(struct ubus_context *ctx, struct ubus_object *obj,
>>   		      struct ubus_request_data *req, const char *method,
>>   		      struct blob_attr *msg)
>>   {
>> +	enum {
>> +		VALIDATION_VALID,
>> +		VALIDATION_FORCEABLE,
>> +		__VALIDATION_MAX
>> +	};
>> +	static const struct blobmsg_policy validation_policy[__VALIDATION_MAX] = {
>> +		[VALIDATION_VALID] = { .name = "valid", .type = BLOBMSG_TYPE_BOOL },
>> +		[VALIDATION_FORCEABLE] = { .name = "forceable", .type = BLOBMSG_TYPE_BOOL },
>> +	};
>> +	struct blob_attr *validation[__VALIDATION_MAX];
>>   	struct blob_attr *tb[__SYSUPGRADE_MAX];
>> +	bool valid, forceable;
>>   
>>   	if (!msg)
>>   		return UBUS_STATUS_INVALID_ARGUMENT;
>> @@ -516,6 +527,19 @@ static int sysupgrade(struct ubus_context *ctx, struct ubus_object *obj,
>>   	if (!tb[SYSUPGRADE_PATH] || !tb[SYSUPGRADE_PREFIX])
>>   		return UBUS_STATUS_INVALID_ARGUMENT;
>>   
>> +	if (validate_firmware_image_call(blobmsg_get_string(tb[SYSUPGRADE_PATH])))
>> +		return UBUS_STATUS_UNKNOWN_ERROR;
>> +
>> +	blobmsg_parse(validation_policy, __VALIDATION_MAX, validation, blob_data(b.head), blob_len(b.head));
>> +
>> +	valid = validation[VALIDATION_VALID] && blobmsg_get_bool(validation[VALIDATION_VALID]);
>> +	forceable = validation[VALIDATION_FORCEABLE] && blobmsg_get_bool(validation[VALIDATION_FORCEABLE]);
>> +
>> +	if (!valid && !forceable) {
>> +		fprintf(stderr, "Firmware image is broken and cannot be installed\n");
>> +		return UBUS_STATUS_UNKNOWN_ERROR;
>> +	}
>> +
>>   	sysupgrade_exec_upgraded(blobmsg_get_string(tb[SYSUPGRADE_PREFIX]),
>>   				 blobmsg_get_string(tb[SYSUPGRADE_PATH]),
>>   				 tb[SYSUPGRADE_COMMAND] ? blobmsg_get_string(tb[SYSUPGRADE_COMMAND]) : NULL,
>> -- 
>> 2.21.0
>>
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel at lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel at lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel


_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel