| Message ID | 20210408052542.3509794-4-dominique.martinet@atmark-techno.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [1/4] archive_handler: make 'missing path attribute' message an ERROR | expand |
On 08.04.21 07:25, Dominique Martinet wrote: > - many ipc_postupdate() calls were done with uninitialized msg, > the function will check data.procmsg.len to decide if it should > memset the buffer or not so at least init this field > (users that send a cleared buffer were left untouched, so these > will clear memory twice) > > - also make sure ipc returned an ACK and not another error, > otherwise errors were lost if the server replied at all. > One user in mongoose did not check return code either, this one > was left untouched > > - swupdate-client: also set exit code to failure if post command > failed > > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > --- > Note: I did not test lua swupdate nor mongoose. > The mongoose ipc_postupdate called that does not check return code > probably should be fixed, I did not take the time to check the > surrounding code to handle it properly. > > swupdate-client, swupdate -i, and swupdate -d were tested in the > following scenarii with all patches applied: > - swu not found: postcommand not run > - swu found, sw-description does not parse: postcommand not run > - swu found, handler fails: postcommand not run > - swu found and correct, postcommand fails: command exit with error > - swu found and correct, postcommand not set: nothing is done, success > - swu found and correct, postcommand ok: runs well. > > bindings/lua_swupdate.c | 3 ++- > corelib/downloader.c | 5 ++--- > mongoose/mongoose_interface.c | 2 +- > tools/swupdate-client.c | 5 ++++- > 4 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/bindings/lua_swupdate.c b/bindings/lua_swupdate.c > index 31721528840c..6778e9f5be71 100644 > --- a/bindings/lua_swupdate.c > +++ b/bindings/lua_swupdate.c > @@ -240,7 +240,8 @@ static int ctrl_close(lua_State *L) { > } > > ipc_message msg; > - if (ipc_postupdate(&msg) != 0) { > + msg.data.procmsg.len = 0; > + if (ipc_postupdate(&msg) != 0 || msg.type != ACK) { > lua_pushnil(L); > lua_pushstring(L, "SWUpdate succeeded but post-update action failed."); > return 2; > diff --git a/corelib/downloader.c b/corelib/downloader.c > index f1fae583d9c2..0638dfa425f5 100644 > --- a/corelib/downloader.c > +++ b/corelib/downloader.c > @@ -145,10 +145,9 @@ int start_download(const char *fname, int argc, char *argv[]) > RECOVERY_STATUS result = download_from_url(&channel_options); > if (result != FAILURE) { > ipc_message msg; > - if (ipc_postupdate(&msg) != 0) { > + msg.data.procmsg.len = 0; > + if (ipc_postupdate(&msg) != 0 || msg.type != ACK) { > result = FAILURE; > - } else { > - result = msg.type == ACK ? result : FAILURE; > } > } > > diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c > index 5dc99f142335..bd5391d61eb9 100644 > --- a/mongoose/mongoose_interface.c > +++ b/mongoose/mongoose_interface.c > @@ -112,7 +112,7 @@ static void restart_handler(struct mg_connection *nc, int ev, void *ev_data) > } > > int ret = ipc_postupdate(&msg); > - if (ret) { > + if (ret || msg.type != ACK) { > mg_http_send_error(nc, 500, "Failed to queue command"); > return; > } > diff --git a/tools/swupdate-client.c b/tools/swupdate-client.c > index 22e55de3794d..c439cf911b6c 100644 > --- a/tools/swupdate-client.c > +++ b/tools/swupdate-client.c > @@ -107,8 +107,11 @@ static int end(RECOVERY_STATUS status) > if (status == SUCCESS && run_postupdate) { > fprintf(stdout, "Executing post-update actions.\n"); > ipc_message msg; > - if (ipc_postupdate(&msg) != 0) > + msg.data.procmsg.len = 0; > + if (ipc_postupdate(&msg) != 0 || msg.type != ACK) { > fprintf(stderr, "Running post-update failed!\n"); > + end_status = EXIT_FAILURE; > + } > } > > pthread_mutex_lock(&mymutex); > Applied to -master, thanks ! Best regards, Stefano Babic
diff --git a/bindings/lua_swupdate.c b/bindings/lua_swupdate.c index 31721528840c..6778e9f5be71 100644 --- a/bindings/lua_swupdate.c +++ b/bindings/lua_swupdate.c @@ -240,7 +240,8 @@ static int ctrl_close(lua_State *L) { } ipc_message msg; - if (ipc_postupdate(&msg) != 0) { + msg.data.procmsg.len = 0; + if (ipc_postupdate(&msg) != 0 || msg.type != ACK) { lua_pushnil(L); lua_pushstring(L, "SWUpdate succeeded but post-update action failed."); return 2; diff --git a/corelib/downloader.c b/corelib/downloader.c index f1fae583d9c2..0638dfa425f5 100644 --- a/corelib/downloader.c +++ b/corelib/downloader.c @@ -145,10 +145,9 @@ int start_download(const char *fname, int argc, char *argv[]) RECOVERY_STATUS result = download_from_url(&channel_options); if (result != FAILURE) { ipc_message msg; - if (ipc_postupdate(&msg) != 0) { + msg.data.procmsg.len = 0; + if (ipc_postupdate(&msg) != 0 || msg.type != ACK) { result = FAILURE; - } else { - result = msg.type == ACK ? result : FAILURE; } } diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c index 5dc99f142335..bd5391d61eb9 100644 --- a/mongoose/mongoose_interface.c +++ b/mongoose/mongoose_interface.c @@ -112,7 +112,7 @@ static void restart_handler(struct mg_connection *nc, int ev, void *ev_data) } int ret = ipc_postupdate(&msg); - if (ret) { + if (ret || msg.type != ACK) { mg_http_send_error(nc, 500, "Failed to queue command"); return; } diff --git a/tools/swupdate-client.c b/tools/swupdate-client.c index 22e55de3794d..c439cf911b6c 100644 --- a/tools/swupdate-client.c +++ b/tools/swupdate-client.c @@ -107,8 +107,11 @@ static int end(RECOVERY_STATUS status) if (status == SUCCESS && run_postupdate) { fprintf(stdout, "Executing post-update actions.\n"); ipc_message msg; - if (ipc_postupdate(&msg) != 0) + msg.data.procmsg.len = 0; + if (ipc_postupdate(&msg) != 0 || msg.type != ACK) { fprintf(stderr, "Running post-update failed!\n"); + end_status = EXIT_FAILURE; + } } pthread_mutex_lock(&mymutex);
- many ipc_postupdate() calls were done with uninitialized msg, the function will check data.procmsg.len to decide if it should memset the buffer or not so at least init this field (users that send a cleared buffer were left untouched, so these will clear memory twice) - also make sure ipc returned an ACK and not another error, otherwise errors were lost if the server replied at all. One user in mongoose did not check return code either, this one was left untouched - swupdate-client: also set exit code to failure if post command failed Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> --- Note: I did not test lua swupdate nor mongoose. The mongoose ipc_postupdate called that does not check return code probably should be fixed, I did not take the time to check the surrounding code to handle it properly. swupdate-client, swupdate -i, and swupdate -d were tested in the following scenarii with all patches applied: - swu not found: postcommand not run - swu found, sw-description does not parse: postcommand not run - swu found, handler fails: postcommand not run - swu found and correct, postcommand fails: command exit with error - swu found and correct, postcommand not set: nothing is done, success - swu found and correct, postcommand ok: runs well. bindings/lua_swupdate.c | 3 ++- corelib/downloader.c | 5 ++--- mongoose/mongoose_interface.c | 2 +- tools/swupdate-client.c | 5 ++++- 4 files changed, 9 insertions(+), 6 deletions(-)