From c332bec1c30fca6c562dee63bb3b5f5793068369 Mon Sep 17 00:00:00 2001 From: panxiaohe Date: Mon, 15 Aug 2022 17:29:10 +0800 Subject: [PATCH] cli: fix memory handling with new popt library --- authselect.spec | 7 +- ...emory-handling-with-new-popt-library.patch | 442 ++++++++++++++++++ ...n-unnecessary-NULL-check-before-free.patch | 36 ++ 3 files changed, 484 insertions(+), 1 deletion(-) create mode 100644 backport-cli-fix-memory-handling-with-new-popt-library.patch create mode 100644 backport-main-Drop-an-unnecessary-NULL-check-before-free.patch diff --git a/authselect.spec b/authselect.spec index 414588a..fc95717 100644 --- a/authselect.spec +++ b/authselect.spec @@ -1,12 +1,14 @@ Name: authselect Version: 1.2.4 -Release: 5 +Release: 6 Summary: A tool to select system authentication and identity sources from a list of supported profiles License: GPLv3+ URL: https://github.com/authselect/authselect Source0: https://github.com/authselect/authselect/archive/%{version}/%{name}-%{version}.tar.gz Patch0: authselect-revert-remove-authselect-compat-package.patch +Patch1: backport-main-Drop-an-unnecessary-NULL-check-before-free.patch +Patch2: backport-cli-fix-memory-handling-with-new-popt-library.patch BuildRequires: autoconf gettext-devel automake libtool popt-devel libcmocka-devel BuildRequires: m4 gcc pkgconfig pkgconfig(popt) po4a asciidoc python3-devel @@ -111,6 +113,9 @@ sed -i -E '/^\w+=$/d' %{_sysconfdir}/security/pwquality.conf.d/10-authconfig-pwq exit 0 %changelog +* Mon Aug 15 2022 panxiaohe - 1.2.4-6 +- cli: fix memory handling with new popt library + * Sun May 15 2022 yixiangzhike - 1.2.4-5 - provide default directory /etc/authselect diff --git a/backport-cli-fix-memory-handling-with-new-popt-library.patch b/backport-cli-fix-memory-handling-with-new-popt-library.patch new file mode 100644 index 0000000..681192a --- /dev/null +++ b/backport-cli-fix-memory-handling-with-new-popt-library.patch @@ -0,0 +1,442 @@ +From 35643637c2964e9dd1a459fd76076b088219c117 Mon Sep 17 00:00:00 2001 +From: Tomas Halman +Date: Tue, 28 Jun 2022 11:07:18 +0200 +Subject: [PATCH] cli: fix memory handling with new popt library + +This patch makes a copy of the string returned by popt so +the string can be safely used after releasing popt context. + +Resolves: https://github.com/authselect/authselect/issues/313 +--- + src/cli/cli_tool.c | 19 +++++++- + src/cli/cli_tool.h | 2 +- + src/cli/main.c | 109 ++++++++++++++++++++++++++++----------------- + 3 files changed, 86 insertions(+), 44 deletions(-) + +diff --git a/src/cli/cli_tool.c b/src/cli/cli_tool.c +index 83bc1ef..7cf0d45 100644 +--- a/src/cli/cli_tool.c ++++ b/src/cli/cli_tool.c +@@ -301,7 +301,7 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline, + void *popt_fn_pvt, + const char *fopt_name, + const char *fopt_help, +- const char **_fopt, ++ char **_fopt, + bool allow_more_free_opts, + bool *_opt_set) + { +@@ -319,6 +319,11 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline, + bool opt_set; + int ret; + ++ /* Set output parameter _fopt to NULL value if present. */ ++ if (_fopt != NULL) { ++ *_fopt = NULL; ++ } ++ + /* Create help option string. We always need to append command name since + * we use POPT_CONTEXT_KEEP_FIRST. */ + if (fopt_name == NULL) { +@@ -379,7 +384,12 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline, + } + } + +- *_fopt = fopt; ++ *_fopt = strdup(fopt); ++ if (*_fopt == NULL) { ++ ERROR("Out of memory!"); ++ ret = ENOMEM; ++ goto done; ++ } + } else if (_fopt == NULL && fopt != NULL) { + /* Unexpected free argument. */ + fprintf(stderr, _("Unexpected parameter: %s\n\n"), fopt); +@@ -410,6 +420,11 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline, + done: + poptFreeContext(pc); + free(help); ++ if (ret != EOK && _fopt != NULL) { ++ free(*_fopt); ++ *_fopt = NULL; ++ } ++ + return ret; + } + +diff --git a/src/cli/cli_tool.h b/src/cli/cli_tool.h +index a52260f..b3b361c 100644 +--- a/src/cli/cli_tool.h ++++ b/src/cli/cli_tool.h +@@ -68,7 +68,7 @@ errno_t cli_tool_popt_ex(struct cli_cmdline *cmdline, + void *popt_fn_pvt, + const char *fopt_name, + const char *fopt_help, +- const char **_fopt, ++ char **_fopt, + bool allow_more_free_opts, + bool *_opt_set); + +diff --git a/src/cli/main.c b/src/cli/main.c +index afe1009..18486b5 100644 +--- a/src/cli/main.c ++++ b/src/cli/main.c +@@ -61,15 +61,17 @@ list_max_length(char **list) + static errno_t + parse_profile_options(struct cli_cmdline *cmdline, + struct poptOption *options, +- const char **_profile_id, ++ char **_profile_id, + const char ***_features) + { +- const char *profile_id; ++ char *profile_id; + const char **features; + bool profile_skipped; + errno_t ret; + int i, j; + ++ *_profile_id = NULL; ++ + ret = cli_tool_popt_ex(cmdline, options, CLI_TOOL_OPT_OPTIONAL, + NULL, NULL, "PROFILE-ID", _("Profile identifier."), + &profile_id, true, NULL); +@@ -80,6 +82,7 @@ parse_profile_options(struct cli_cmdline *cmdline, + + features = malloc_zero_array(const char *, cmdline->argc); + if (features == NULL) { ++ free(profile_id); + return ENOMEM; + } + +@@ -143,7 +146,7 @@ static errno_t activate(struct cli_cmdline *cmdline) + { + struct authselect_profile *profile = NULL; + const char **features = NULL; +- const char *profile_id; ++ char *profile_id = NULL; + char *requirements = NULL; + char *backup_name = NULL; + char **maps = NULL; +@@ -232,6 +235,7 @@ done: + authselect_array_free(maps); + authselect_profile_free(profile); + free(features); ++ free(profile_id); + + return ret; + } +@@ -428,7 +432,7 @@ done: + static errno_t list_features(struct cli_cmdline *cmdline) + { + struct authselect_profile *profile; +- const char *profile_id; ++ char *profile_id; + char **features; + errno_t ret; + int i; +@@ -438,14 +442,14 @@ static errno_t list_features(struct cli_cmdline *cmdline) + &profile_id, true, NULL); + if (ret != EOK) { + ERROR("Unable to parse command arguments"); +- return ret; ++ goto done; + } + + ret = authselect_profile(profile_id, &profile); + if (ret != EOK) { + ERROR("Unable to get profile information [%d]: %s", + ret, strerror(ret)); +- return ret; ++ goto done; + } + + features = authselect_profile_features(profile); +@@ -453,7 +457,8 @@ static errno_t list_features(struct cli_cmdline *cmdline) + if (features == NULL) { + ERROR("Unable to get profile features [%d]: %s", + ret, strerror(ret)); +- return ENOMEM; ++ ret = ENOMEM; ++ goto done; + } + + for (i = 0; features[i] != NULL; i++) { +@@ -462,13 +467,17 @@ static errno_t list_features(struct cli_cmdline *cmdline) + + authselect_array_free(features); + +- return EOK; ++ ret = EOK; ++ ++done: ++ free(profile_id); ++ return ret; + } + + static errno_t show(struct cli_cmdline *cmdline) + { + struct authselect_profile *profile; +- const char *profile_id; ++ char *profile_id; + errno_t ret; + + ret = cli_tool_popt_ex(cmdline, NULL, CLI_TOOL_OPT_OPTIONAL, +@@ -476,41 +485,47 @@ static errno_t show(struct cli_cmdline *cmdline) + &profile_id, false, NULL); + if (ret != EOK) { + ERROR("Unable to parse command arguments"); +- return ret; ++ goto done; + } + + ret = authselect_profile(profile_id, &profile); + if (ret != EOK) { + ERROR("Unable to get profile information [%d]: %s", + ret, strerror(ret)); +- return ENOMEM; ++ ret = ENOMEM; ++ goto done; + } + + puts(authselect_profile_description(profile)); + + authselect_profile_free(profile); + +- return EOK; ++ ret = EOK; ++ ++done: ++ free(profile_id); ++ return ret; + } + + static errno_t requirements(struct cli_cmdline *cmdline) + { +- struct authselect_profile *profile; +- const char *profile_id; ++ struct authselect_profile *profile = NULL; ++ char *profile_id = NULL; + const char **features; +- char *requirements; ++ char *requirements = NULL; + errno_t ret; + + ret = parse_profile_options(cmdline, NULL, &profile_id, &features); + if (ret != EOK) { +- return ret; ++ goto done; + } + + ret = authselect_profile(profile_id, &profile); + if (ret != EOK) { + ERROR("Unable to get profile information [%d]: %s", + ret, strerror(ret)); +- return ENOMEM; ++ ret = ENOMEM; ++ goto done; + } + + requirements = authselect_profile_requirements(profile, features); +@@ -528,6 +543,7 @@ static errno_t requirements(struct cli_cmdline *cmdline) + + done: + free(requirements); ++ free(profile_id); + authselect_profile_free(profile); + + return ret; +@@ -536,7 +552,7 @@ done: + static errno_t test(struct cli_cmdline *cmdline) + { + struct authselect_files *files; +- const char *profile_id; ++ char *profile_id = NULL; + const char **features; + const char *content; + const char *path; +@@ -583,13 +599,13 @@ static errno_t test(struct cli_cmdline *cmdline) + + ret = parse_profile_options(cmdline, options, &profile_id, &features); + if (ret != EOK) { +- return ret; ++ goto done; + } + + ret = authselect_files(profile_id, features, &files); + if (ret != EOK) { + ERROR("Unable to get generated content [%d]: %s", ret, strerror(ret)); +- return ret; ++ goto done; + } + + for (i = 0; generated[i].content_fn != NULL; i++) { +@@ -613,7 +629,9 @@ static errno_t test(struct cli_cmdline *cmdline) + } + } + +- return EOK; ++done: ++ free(profile_id); ++ return ret; + } + + static errno_t enable(struct cli_cmdline *cmdline) +@@ -622,7 +640,7 @@ static errno_t enable(struct cli_cmdline *cmdline) + char *backup_name = NULL; + char *requirements = NULL; + char *profile_id = NULL; +- const char *feature; ++ char *feature; + const char *features[2]; + int backup = 0; + int quiet = 0; +@@ -693,6 +711,7 @@ static errno_t enable(struct cli_cmdline *cmdline) + done: + free(profile_id); + free(requirements); ++ free(feature); + authselect_profile_free(profile); + + return ret; +@@ -702,7 +721,7 @@ static errno_t disable(struct cli_cmdline *cmdline) + { + int backup = 0; + char *backup_name = NULL; +- const char *feature; ++ char *feature; + errno_t ret; + + struct poptOption options[] = { +@@ -716,32 +735,34 @@ static errno_t disable(struct cli_cmdline *cmdline) + &feature, false, NULL); + if (ret != EOK) { + ERROR("Unable to parse command arguments"); +- return ret; ++ goto done; + } + + ret = perform_backup(false, backup, backup_name); + if (ret != EOK) { +- return ret; ++ goto done; + } + + ret = authselect_feature_disable(feature); + if (ret != EOK) { + CLI_ERROR("Unable to disable feature [%d]: %s\n", ret, strerror(ret)); +- return ret; ++ goto done; + } + +- return EOK; ++done: ++ free(feature); ++ return ret; + } + + static errno_t create(struct cli_cmdline *cmdline) + { +- const char *name; ++ char *name; + const char *base_id = NULL; + enum authselect_profile_type type = AUTHSELECT_PROFILE_CUSTOM; + enum authselect_profile_type base_type = AUTHSELECT_PROFILE_ANY; + int symlink_flags = AUTHSELECT_SYMLINK_NONE; + const char **symlinks = NULL; +- char *path; ++ char *path = NULL; + errno_t ret; + + struct poptOption options[] = { +@@ -761,20 +782,22 @@ static errno_t create(struct cli_cmdline *cmdline) + &name, false, NULL); + if (ret != EOK) { + ERROR("Unable to parse command arguments"); +- return ret; ++ goto done; + } + + ret = authselect_profile_create(name, type, base_id, base_type, + symlink_flags, symlinks, &path); + if (ret != EOK) { + CLI_ERROR("Unable to create new profile [%d]: %s\n", ret, strerror(ret)); +- return ret; ++ goto done; + } + + CLI_PRINT("New profile was created at %s\n", path); +- free(path); + +- return EOK; ++done: ++ free(path); ++ free(name); ++ return ret; + } + + static errno_t backup_list(struct cli_cmdline *cmdline) +@@ -855,7 +878,7 @@ done: + + static errno_t backup_remove(struct cli_cmdline *cmdline) + { +- const char *name; ++ char *name; + errno_t ret; + + ret = cli_tool_popt_ex(cmdline, NULL, CLI_TOOL_OPT_OPTIONAL, +@@ -864,22 +887,24 @@ static errno_t backup_remove(struct cli_cmdline *cmdline) + &name, false, NULL); + if (ret != EOK) { + ERROR("Unable to parse command arguments"); +- return ret; ++ goto done; + } + + ret = authselect_backup_remove(name); + if (ret != EOK) { + CLI_ERROR("Unable to remove backup [%s] [%d]: %s\n", + name, ret, strerror(ret)); +- return ret; ++ goto done; + } + +- return EOK; ++done: ++ free(name); ++ return ret; + } + + static errno_t backup_restore(struct cli_cmdline *cmdline) + { +- const char *name; ++ char *name; + errno_t ret; + + ret = cli_tool_popt_ex(cmdline, NULL, CLI_TOOL_OPT_OPTIONAL, +@@ -888,17 +913,19 @@ static errno_t backup_restore(struct cli_cmdline *cmdline) + &name, false, NULL); + if (ret != EOK) { + ERROR("Unable to parse command arguments"); +- return ret; ++ goto done; + } + + ret = authselect_backup_restore(name); + if (ret != EOK) { + CLI_ERROR("Unable to restore backup [%s] [%d]: %s\n", + name, ret, strerror(ret)); +- return ret; ++ goto done; + } + +- return EOK; ++done: ++ free(name); ++ return ret; + } + + static errno_t uninstall(struct cli_cmdline *cmdline) +-- +2.27.0 + diff --git a/backport-main-Drop-an-unnecessary-NULL-check-before-free.patch b/backport-main-Drop-an-unnecessary-NULL-check-before-free.patch new file mode 100644 index 0000000..a2519e1 --- /dev/null +++ b/backport-main-Drop-an-unnecessary-NULL-check-before-free.patch @@ -0,0 +1,36 @@ +From 46386b75fb90ce91ede80093ce73e99fde53ba3b Mon Sep 17 00:00:00 2001 +From: Colin Walters +Date: Tue, 4 Jan 2022 18:33:30 -0500 +Subject: [PATCH] main: Drop an unnecessary `NULL` check before `free()` + +From `man free()`: + +``` +The free() function frees the memory space pointed to by ptr ... If ptr is NULL, no operation is performed. +``` + +Obviously there are *tons* of these in the codebase; just doing +this one as a preliminary PR; if accepted I may do some more, or +others can. Or we could try a coccinelle semantic patch. +--- + src/cli/main.c | 4 +--- + 1 file changed, 1 insertion(+), 3 deletions(-) + +diff --git a/src/cli/main.c b/src/cli/main.c +index 4b8ab85..575e56f 100644 +--- a/src/cli/main.c ++++ b/src/cli/main.c +@@ -231,9 +231,7 @@ done: + free(requirements); + authselect_array_free(maps); + authselect_profile_free(profile); +- if (features != NULL) { +- free(features); +- } ++ free(features); + + return ret; + } +-- +2.27.0 +