From 86f6156305db2089de4963cec0e41ee2afdd188e Mon Sep 17 00:00:00 2001 From: Adam Michalski Date: Fri, 22 Dec 2023 16:08:41 +0100 Subject: [PATCH] Fix SVACE and Coverity issues Co-author: Mateusz Majewski Change-Id: I6811590979486570f564b332398e6cd0ed8565c3 --- packaging/isu.spec | 2 +- src/libisu/libisu-internal.c | 76 ++++++++++++++++++++++++++++++-------------- src/libisu/libisu.c | 7 ++-- src/pkg_manager/isu.c | 1 + 4 files changed, 57 insertions(+), 29 deletions(-) diff --git a/packaging/isu.spec b/packaging/isu.spec index 7944826..67b8afe 100644 --- a/packaging/isu.spec +++ b/packaging/isu.spec @@ -1,7 +1,7 @@ %define libisu_version 0.0.1 Summary: Individual Service Upgrade support Name: isu -Version: 9.0.4 +Version: 9.0.4.1 Release: 1 Source0: %{name}-%{version}.tar.gz License: MIT diff --git a/src/libisu/libisu-internal.c b/src/libisu/libisu-internal.c index 9f41195..5295511 100644 --- a/src/libisu/libisu-internal.c +++ b/src/libisu/libisu-internal.c @@ -58,6 +58,16 @@ bool is_isu_feature_supported() #endif } +static void cleanup_libmnt_table(struct libmnt_table **table) +{ + mnt_unref_table(*table); +} + +static void cleanup_char_ptr(char **ptr) +{ + free(*ptr); +} + static int copy_content(const char *src, const char *dst, const char **exclude) { int result = -1; @@ -105,19 +115,28 @@ static int copy_content(const char *src, const char *dst, const char **exclude) int src_file = open(newsrc, O_RDONLY); int dst_file = open(newdst, O_CREAT | O_WRONLY, mode); if (src_file == -1 || dst_file == -1) { - if (src_file == -1) - SLOGE("Cannot open file %s: (%d) %m", newsrc, errno); - if (dst_file == -1) + if (src_file != -1) { SLOGE("Cannot open file %s: (%d) %m", newdst, errno); + close(src_file); + } + if (dst_file != -1) { + SLOGE("Cannot open file %s: (%d) %m", newsrc, errno); + close(dst_file); + } goto exit; } struct stat fileinfo = {0}; if (fstat(src_file, &fileinfo) == -1) { SLOGE("Get file %s status error: (%d) %m", newsrc, errno); + close(src_file); + close(dst_file); + goto exit; } if (sendfile(dst_file, src_file, NULL, fileinfo.st_size) == -1) { SLOGE("Copy file content (%s -> %s) error: (%d) %m", newsrc, newdst, errno); + close(src_file); + close(dst_file); goto exit; } } @@ -185,17 +204,32 @@ exit: return res; } -static void fill_system_services(char *line, struct _isu_pkg_info *pkg_info) +static bool fill_system_services(char *line, struct _isu_pkg_info *pkg_info) { char *saveptr; char *token = strtok_r(line, " ", &saveptr); while (token != NULL) { + __attribute__ ((__cleanup__(cleanup_char_ptr))) char *token_dup = strdup(token); + if (token_dup == NULL) { + SLOGE("Memory allocation error (%d): %m\n", errno); + return false; + } + + char **service_files_re = realloc(pkg_info->service_files, + (pkg_info->service_files_len+1)*sizeof(char*)); + if (service_files_re == NULL) { + SLOGE("Memory allocation error (%d): %m\n", errno); + return false; + } + + pkg_info->service_files = service_files_re; pkg_info->service_files_len++; - pkg_info->service_files = realloc(pkg_info->service_files, - pkg_info->service_files_len*sizeof(char*)); - pkg_info->service_files[pkg_info->service_files_len-1] = strdup(token); + pkg_info->service_files[pkg_info->service_files_len-1] = token_dup; + token_dup = NULL; + token = strtok_r(NULL, " ", &saveptr); } + return true; } static int copy_data(struct archive *ar, struct archive *aw) @@ -370,6 +404,7 @@ struct _isu_pkg_info* get_pkg_info(const char *isu_cfg_path) char *lineptr = NULL; size_t n = 0; + bool fill_ok = true; while (getline(&lineptr, &n, f) > 0) { char *line = lineptr; while (line && line[0] == ' ') { line++; }; @@ -389,16 +424,16 @@ struct _isu_pkg_info* get_pkg_info(const char *isu_cfg_path) value[val_len-1] = '\0'; } - if (strncmp(token, CFG_NAME, strlen(CFG_NAME)) == 0) { + if (strncmp(token, CFG_NAME, strlen(CFG_NAME)) == 0 && pkg_info->name == NULL) { pkg_info->name = strdup(value); - } else if (strncmp(token, CFG_VERSION, strlen(CFG_VERSION)) == 0) { + } else if (strncmp(token, CFG_VERSION, strlen(CFG_VERSION)) == 0 && pkg_info->version == NULL) { pkg_info->version = strdup(value); } else if (strncmp(token, CFG_SYSTEM_SERVICE, strlen(CFG_SYSTEM_SERVICE)) == 0) { - fill_system_services(value, pkg_info); + fill_ok &= fill_system_services(value, pkg_info); } } - if (!pkg_info->name || !pkg_info->version) { + if (!pkg_info->name || !pkg_info->version || !fill_ok) { SLOGE("Cannot read content of isu.cfg"); isu_pkg_info_free(pkg_info); pkg_info = NULL; @@ -513,16 +548,6 @@ isu_result isu_uninstall_internal(const char *name) return result; } -void cleanup_libmnt_table(struct libmnt_table **table) -{ - mnt_unref_table(*table); -} - -void cleanup_char_ptr(char **ptr) -{ - free(*ptr); -} - isu_file_res is_isu_file(const char *path, pid_t pid, char **source_path) { if (!is_isu_feature_supported()) @@ -542,6 +567,11 @@ isu_file_res is_isu_file(const char *path, pid_t pid, char **source_path) } __attribute__ ((__cleanup__(cleanup_char_ptr))) char *fname_orig = mnt_resolve_path(path, NULL); + if (fname_orig == NULL) { + // N.B. mnt_resolve_path does not set errno. It only returns an absolute path or NULL in case of error + SLOGE("mnt_resolve_path error\n"); + return ISU_FILE_RES_ERROR; + } /* * Check if the fname_orig file or it subpath is bindmounted @@ -582,9 +612,9 @@ isu_file_res is_isu_file(const char *path, pid_t pid, char **source_path) /* * If so, get the path in the mounted image */ - char *in_image = strdup(mnt_fs_get_root(fs)); + const char *in_image = mnt_fs_get_root(fs); if (in_image == NULL) { - SLOGE("Memory allocation error"); + SLOGE("mnt_fs_get_root error"); return ISU_FILE_RES_ERROR; } diff --git a/src/libisu/libisu.c b/src/libisu/libisu.c index 4270c4e..11554c1 100644 --- a/src/libisu/libisu.c +++ b/src/libisu/libisu.c @@ -62,6 +62,7 @@ isu_pkg_list isu_list_init() DIR *dir = opendir(ISU_PKG_PATH); if (dir == NULL) { SLOGE("Open dir %s error (%d): %m\n", ISU_PKG_PATH, errno); + isu_list_free(pkg_list); return NULL; } @@ -184,10 +185,6 @@ isu_result isu_pkg_get_version(isu_pkg_info pkg_info, char *version, size_t len) isu_result isu_pkg_info_free(isu_pkg_info pkg_info) { RET_IF_FEATURE_NOT_SUPPORTED(ISU_RES_ERR_FEATURE); - if (pkg_info == NULL) { - goto exit; - } - if (pkg_info == NULL) return ISU_RES_ERR_ARGUMENT; @@ -203,7 +200,7 @@ isu_result isu_pkg_info_free(isu_pkg_info pkg_info) free(pkg_info_i->name); free(pkg_info_i->version); free(pkg_info_i); -exit: + return ISU_RES_OK; } diff --git a/src/pkg_manager/isu.c b/src/pkg_manager/isu.c index 3471e77..bf4f32c 100644 --- a/src/pkg_manager/isu.c +++ b/src/pkg_manager/isu.c @@ -174,6 +174,7 @@ static bool print_status_for_package(GDBusConnection *conn, struct _isu_pkg_list char *service_file = info->service_files[i]; if (service_file == NULL) { fprintf(stderr, "Can not get service file for package %s\n", info->name); + isu_pkg_info_free(info); return false; } FREE_AND_NULL_IF_NOT_NULL(unit_name) -- 2.7.4