From 9a74fc30d896c548b6fd2d5eb34836dc2f13672d Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Thu, 30 May 2024 17:20:07 +0900 Subject: [PATCH] ustrtoull callers cleanup: add warnings/errors when ustrotull fails Many callers of ustrtoull were not checking for errors. Since there are legacy SWUs that might depend on them, many of these place should not start erroring out immediately, but we must start somewhere: - add many warnings when ustrtoull failed, without impacting the final result - for the ones that are the most user facing (command line arguments), make it a hard error Signed-off-by: Dominique Martinet Acked-by: Stefano Babic --- corelib/server_utils.c | 6 +++++- handlers/copy_handler.c | 10 ++++++++-- handlers/delta_handler.c | 7 +++++-- handlers/diskpart_handler.c | 4 ++++ suricatta/server_general.c | 5 +++++ suricatta/server_hawkbit.c | 8 ++++++++ suricatta/server_lua.c | 3 +++ 7 files changed, 38 insertions(+), 5 deletions(-) diff --git a/corelib/server_utils.c b/corelib/server_utils.c index 2b3c64e3..9c9265e2 100644 --- a/corelib/server_utils.c +++ b/corelib/server_utils.c @@ -4,6 +4,7 @@ * * SPDX-License-Identifier: GPL-2.0-only */ +#include #include #include #include @@ -24,8 +25,11 @@ int channel_settings(void *elem, void *data) &chan->retries); GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "max-download-speed", tmp); - if (strlen(tmp)) + if (strlen(tmp)) { chan->max_download_speed = (unsigned int)ustrtoull(tmp, NULL, 10); + if (errno) + WARN("max-download-speed setting %s: ustrtoull failed", tmp); + } GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "retrywait", tmp); if (strlen(tmp)) diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c index abf65bd3..3e2f1f40 100644 --- a/handlers/copy_handler.c +++ b/handlers/copy_handler.c @@ -204,9 +204,10 @@ static int recurse_directory(const char *fpath, const struct stat *sb, static int copy_image_file(struct img_type *img, void *data) { int ret = 0; + char *tmp; struct dict_list *proplist; struct dict_list_elem *entry; - size_t size; + size_t size = 0; struct script_handler_data *script_data; bool recursive, createdest; @@ -249,7 +250,12 @@ static int copy_image_file(struct img_type *img, void *data) /* * Detect the size if not set in sw-descriptiont */ - size = dict_get_value(&img->properties, "size") ? ustrtoull(dict_get_value(&img->properties, "size"), NULL, 0) : 0; + tmp = dict_get_value(&img->properties, "size"); + if (tmp) { + size = ustrtoull(tmp, NULL, 0); + if (errno) + WARN("size property %s: ustrtoull failed", tmp); + } /* * No chain set, fallback to rawcopy diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c index 89160036..8e7faa0e 100644 --- a/handlers/delta_handler.c +++ b/handlers/delta_handler.c @@ -326,10 +326,13 @@ static int delta_retrieve_attributes(struct img_type *img, struct hnd_priv *priv char *srcsize; srcsize = dict_get_value(&img->properties, "source-size"); if (srcsize) { - if (!strcmp(srcsize, "detect")) + if (!strcmp(srcsize, "detect")) { priv->detectsrcsize = true; - else + } else { priv->srcsize = ustrtoull(srcsize, NULL, 10); + if (errno) + WARN("source-size %s: ustrotull failed", srcsize); + } } char *zckloglevel = dict_get_value(&img->properties, "zckloglevel"); diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c index 4c2b902b..982c494e 100644 --- a/handlers/diskpart_handler.c +++ b/handlers/diskpart_handler.c @@ -1232,11 +1232,15 @@ static int diskpart(struct img_type *img, switch (i) { case PART_SIZE: part->size = ustrtoull(equal, NULL, 10); + if (errno) + WARN("partition size %s: ustrtoull failed", equal); if (!size_delimiter_match(equal)) part->explicit_size = 1; break; case PART_START: part->start = ustrtoull(equal, NULL, 10); + if (errno) + WARN("partition size %s: ustrtoull failed", equal); break; case PART_TYPE: strncpy(part->type, equal, sizeof(part->type)); diff --git a/suricatta/server_general.c b/suricatta/server_general.c index 5c51fda6..db5a60ae 100644 --- a/suricatta/server_general.c +++ b/suricatta/server_general.c @@ -663,6 +663,11 @@ static server_op_res_t server_start(const char *fname, int argc, char *argv[]) case 'n': channel_data_defaults.max_download_speed = (unsigned int)ustrtoull(optarg, NULL, 10); + if (errno) { + ERROR("max-download-speed %s: ustrtoull failed", + optarg); + return SERVER_EINIT; + } break; /* Ignore the --server option which is already parsed by the caller. */ diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c index 3c38ea61..9c3e9741 100644 --- a/suricatta/server_hawkbit.c +++ b/suricatta/server_hawkbit.c @@ -871,6 +871,9 @@ static void get_action_id_from_env(int *action_id) char *action_str = swupdate_vars_get("action_id", NULL); if (action_str) { int tmp = ustrtoull(action_str, NULL, 10); + if (errno) + WARN("action_id %s: ustrtoull failed", + action_str); /* * action_id = 0 is invalid, then check it */ @@ -1910,6 +1913,11 @@ static server_op_res_t server_start(const char *fname, int argc, char *argv[]) case 'n': channel_data_defaults.max_download_speed = (unsigned int)ustrtoull(optarg, NULL, 10); + if (errno) { + ERROR("max-download-speed %s: ustrtoull failed", + optarg); + return SERVER_EINIT; + } break; /* Ignore not recognized options, they can be already parsed by the caller */ case '?': diff --git a/suricatta/server_lua.c b/suricatta/server_lua.c index f5b90f6d..c749c1c5 100644 --- a/suricatta/server_lua.c +++ b/suricatta/server_lua.c @@ -591,6 +591,9 @@ static void channel_set_options(lua_State *L, channel_data_t *channel_data) get_from_table(L, "max_download_speed", max_download_speed); if (max_download_speed) { channel_data->max_download_speed = (unsigned int)ustrtoull(max_download_speed, NULL, 10); + if (errno) + WARN("max-download-speed %s: ustrtoull failed", + max_download_speed); free(max_download_speed); } lua_getfield(L, -1, "proxy");