[PATCH] hawkbit: fix leak in server_configuration_ipc/server_activation_ipc

[Dominique Martinet]

json_root is never freed in both functions, add the missing
json_object_put() call to free it

server_activation_ipc()'s "result" initial value was changed to allow
straight goto cleanup without setting result on every error path: the
value is set inconditionally after allocating details and before any
pre-existing cleanup call, so the previous default value was not
actually used anywhere.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---
I was a bit worried about the goto because some C revisions don't allow
declaring variables after the first goto, but we're doing similar
variable decarations after goto in e.g. server_get_device_info() so it's
probably fine

These two I just spotted by chance but we didn't run swupdate+hawkbit in
valgrind (yet), this might be worth doing at some point...
(I also briefly checked I didn't introduce any new use-after-free, e.g.
dict_set_value properly strdups the key/values, but I might have missed
some so that's another reason to do it!)


suricatta/server_hawkbit.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
index 781c023a947b..63353f75954b 100644
--- a/suricatta/server_hawkbit.c
+++ b/suricatta/server_hawkbit.c
@@ -1273,7 +1273,7 @@ server_op_res_t server_process_update_artifact(int action_id,
artifact->url);

channel_data_t channel_data = channel_data_defaults;
- channel_data.url =
+ channel_data.url =
strdup(artifact->url);

static const char* const update_info = STRINGIFY(
@@ -2096,9 +2096,10 @@ static server_op_res_t server_stop(void)

static server_op_res_t server_activation_ipc(ipc_message *msg)
{
- server_op_res_t result = SERVER_OK;
+ server_op_res_t result = SERVER_EERR;
update_state_t update_state = STATE_NOT_AVAILABLE;
struct json_object *json_root;
+ const char **details = NULL;

json_root = server_tokenize_msg(msg->data.procmsg.buf,
sizeof(msg->data.procmsg.buf));
@@ -2118,7 +2119,7 @@ static server_op_res_t server_activation_ipc(ipc_message *msg)

if (action_id <= 0) {
ERROR("No action_id passed into JSON message and no action:_id in env");
- return SERVER_EERR;
+ goto cleanup;
}

json_data = json_get_path_key(
@@ -2126,7 +2127,7 @@ static server_op_res_t server_activation_ipc(ipc_message *msg)
if (json_data == NULL) {
ERROR("Got malformed JSON: Could not find field status");
DEBUG("Got JSON: %s", json_object_to_json_string(json_data));
- return SERVER_EERR;
+ goto cleanup;
}
update_state = (unsigned int)*json_object_get_string(json_data);
DEBUG("Got action_id %d status %c", action_id, update_state);
@@ -2140,17 +2141,17 @@ static server_op_res_t server_activation_ipc(ipc_message *msg)
!is_valid_state(update_state)) {
ERROR("Wrong values \"execution\" : %s, \"finished\" : %s , \"status\" : %c",
reply_execution, reply_result, update_state);
- return SERVER_EERR;
+ goto cleanup;
}

if (!json_data) {
ERROR("No details are passed, they are mandatory.");
- return SERVER_EERR;
+ goto cleanup;
}
int numdetails = json_object_array_length(json_data);
- const char **details = (const char **)malloc((numdetails + 1) * (sizeof (char *)));
- if(!details)
- return SERVER_EERR;
+ details = (const char **)malloc((numdetails + 1) * (sizeof (char *)));
+ if (!details)
+ goto cleanup;

if (!numdetails)
details[0] = "";
@@ -2214,6 +2215,7 @@ static server_op_res_t server_activation_ipc(ipc_message *msg)

cleanup:
free(details);
+ json_object_put(json_root);

return result;
}
@@ -2263,6 +2265,7 @@ static server_op_res_t server_configuration_ipc(ipc_message *msg)
}

pthread_mutex_unlock(&ipc_lock);
+ json_object_put(json_root);
return SERVER_OK;
}

--
2.55.0.dirty

[Dominique Martinet]

Dominique Martinet wrote on Tue, Sep 15, 2026 at 03:19:53PM +0900:
> These two I just spotted by chance but we didn't run swupdate+hawkbit in
> valgrind (yet), this might be worth doing at some point...
> (I also briefly checked I didn't introduce any new use-after-free, e.g.
> dict_set_value properly strdups the key/values, but I might have missed
> some so that's another reason to do it!)

FWIW I ran valgrind quickly and this was fine, but there seem to be
(a few other leaks in or around the server_hawkbit code, so we'll look
at it a bit more closely in the next few weeks -- but next week is
mostly holidays in Japan so the patches might slip until October
depending on how it fares.

(Just a heads up, not expecting anything here)

--
Dominique