BUG: post-update command is no longer called from mongoose interface

[Michael Glembotzki]

Hi all,

the post-update command is no longer called when running an update via the integrated mongoose webserver. This "bug" was probably introduced by [1].

The mongoose webserver layer has its own state machine, which is fed by the swupdate notifications sent to it. When SWUpdate sends a message starting the update was successful, the ipc_postupdate() function [2] is triggered. But somehow, the "source" was no longer set to SOURCE_WEBSERVER, so this line is skipped. The post update command will not run.

In my tests, the bug only occurs if the swu file is greater than 100kB, what causes [3] to be called.

[1] https://github.com/sbabic/swupdate/commit/d116870e943de2af088e09ccb8cce71aac7be2eb
[2] https://github.com/sbabic/swupdate/blob/master/mongoose/mongoose_interface.c#L492
[3] https://github.com/sbabic/swupdate/blob/master/mongoose/mongoose_interface.c#L640

Reported-by: Erik Schumacher <erik.sc...@iris-sensing.com>
Reported-by: Michael Glembotzki <Michael.G...@iris-sensing.com>

Best Regards,
Michael

[Storm, Christian]

Hi,

Yes, I can confirm this issue. The (deeper) problem is also mentioned in the commit message of [1], quote:
"core/progress_thread.c's `static struct swupdate_progress progress` is stateful so source is not reset after download progress reporting."

Hence, I do see the following options:
(1) mongoose doesn't check for msg.source == SOURCE_WEBSERVER (if it's the only currently active ingress channel, this would be OK),
(2) keeping the state of msg.source is if-then-else'd, or
(3) state keeping is removed altogether from `static struct swupdate_progress progress`.



Kind regards,
Christian

--
Dr. Christian Storm
Siemens AG, Technology, T CED OES-DE
Otto-Hahn-Ring 6, 81739 Munich, Germany

[Storm, Christian]

After having skimmed through the source again, I think we can go with (1) as the others, e.g.,
https://github.com/sbabic/swupdate/blob/master/tools/swupdate-progress.c#L431
https://github.com/sbabic/swupdate/blob/master/tools/swupdate-client.c#L110
https://github.com/sbabic/swupdate/blob/master/core/install_from_file.c#L59
https://github.com/sbabic/swupdate/blob/master/tools/swupdate-ipc.c#L644
https://github.com/sbabic/swupdate/blob/master/corelib/downloader.c#L81
are also not checking for .source == <something>

Note that not checking for .source does mean that if you have, e.g., mongoose and swupdate-progress running, *both* may take action, so one needs to make sure there's only *one* acting on this "signal"...

[Stefano Babic]

Hi Christian, Michael,

On 08.01.24 02:50, 'Storm, Christian' via swupdate wrote:
> Hi,
>
>> the post-update command is no longer called when running an update via the integrated mongoose webserver. This "bug" was probably introduced by [1].
>>
>> The mongoose webserver layer has its own state machine, which is fed by the swupdate notifications sent to it. When SWUpdate sends a message starting the update was successful, the ipc_postupdate() function [2] is triggered. But somehow, the "source" was no longer set to SOURCE_WEBSERVER, so this line is skipped. The post update command will not run.
>>
>> In my tests, the bug only occurs if the swu file is greater than 100kB, what causes [3] to be called.
>>
>> [1] https://github.com/sbabic/swupdate/commit/d116870e943de2af088e09ccb8cce71aac7be2eb
>> [2] https://github.com/sbabic/swupdate/blob/master/mongoose/mongoose_interface.c#L492
>> [3] https://github.com/sbabic/swupdate/blob/master/mongoose/mongoose_interface.c#L640
>>
>> Reported-by: Erik Schumacher <erik.sc...@iris-sensing.com>
>> Reported-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
>
> Yes, I can confirm this issue. The (deeper) problem is also mentioned in the commit message of [1], quote:
> "core/progress_thread.c's `static struct swupdate_progress progress` is stateful so source is not reset after download progress reporting."
>
> Hence, I do see the following options:
> (1) mongoose doesn't check for msg.source == SOURCE_WEBSERVER (if it's the only currently active ingress channel, this would be OK),

But SWUpdate supports update from multiple interface. Doing this, the
postupdate is called even if the Webserver is not the source.

The code in Webserver is an exception compared to the other interfaces.
On other interface, postupdate is called by the task pushing the SWU,
and getting the source is not required. In case of the Webserver,
mongoose callbacks are used and the check is done by a separate thread
reading from the progress interface, and this is not aware of the source.

> (2) keeping the state of msg.source is if-then-else'd, or

Yes, the Webserver interface should cache if the update is coming from
Webserver, and running postupdate just in that case.

> (3) state keeping is removed altogether from `static struct swupdate_progress progress`.
>
>

Best regards,
Stefano

>
> Kind regards,
> Christian
>