From cb8d3f75960a5966056d9bd9c000b3adbe2d15a3 Mon Sep 17 00:00:00 2001 From: Ian Kent Date: Wed, 28 Jul 2021 13:59:06 +0800 Subject: [PATCH] - add changes for bug 1984808. --- ...1.7-eliminate-some-more-alloca-usage.patch | 197 ++++++++++++++++++ ....7-fix-concat_options-error-handling.patch | 124 +++++++++++ ...7-use-default-stack-size-for-threads.patch | 128 ++++++++++++ autofs.spec | 13 +- 4 files changed, 461 insertions(+), 1 deletion(-) create mode 100644 autofs-5.1.7-eliminate-some-more-alloca-usage.patch create mode 100644 autofs-5.1.7-fix-concat_options-error-handling.patch create mode 100644 autofs-5.1.7-use-default-stack-size-for-threads.patch diff --git a/autofs-5.1.7-eliminate-some-more-alloca-usage.patch b/autofs-5.1.7-eliminate-some-more-alloca-usage.patch new file mode 100644 index 0000000..5f07a22 --- /dev/null +++ b/autofs-5.1.7-eliminate-some-more-alloca-usage.patch @@ -0,0 +1,197 @@ +autofs-5.1.7 - eliminate some more alloca usage + +From: Ian Kent + +Quite a bit of the alloca(3) usage has been eliminated over time. +Use malloc(3) for some more cases that might need to allocate a largish +amount of storage. + +Signed-off-by: Ian Kent +--- + CHANGELOG | 1 + + modules/lookup_program.c | 11 ++++++++++- + modules/lookup_yp.c | 22 +++++++++++++++++++--- + modules/parse_sun.c | 13 +++++++++++-- + modules/replicated.c | 15 ++++----------- + 5 files changed, 45 insertions(+), 17 deletions(-) + +diff --git a/CHANGELOG b/CHANGELOG +index 8d050552..2b7cfaa0 100644 +--- a/CHANGELOG ++++ b/CHANGELOG +@@ -79,6 +79,7 @@ + - add missing description of null map option. + - fix nonstrict offset mount fail handling. + - fix concat_options() error handling. ++- eliminate some more alloca usage. + + 25/01/2021 autofs-5.1.7 + - make bind mounts propagation slave by default. +diff --git a/modules/lookup_program.c b/modules/lookup_program.c +index 6cab52c8..028580e5 100644 +--- a/modules/lookup_program.c ++++ b/modules/lookup_program.c +@@ -636,7 +636,14 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void * + char *ent = NULL; + + if (me->mapent) { +- ent = alloca(strlen(me->mapent) + 1); ++ ent = malloc(strlen(me->mapent) + 1); ++ if (!ent) { ++ char buf[MAX_ERR_BUF]; ++ char *estr = strerror_r(errno, buf, MAX_ERR_BUF); ++ error(ap->logopt, MODPREFIX "malloc: %s", estr); ++ cache_unlock(mc); ++ goto out_free; ++ } + strcpy(ent, me->mapent); + } + cache_unlock(mc); +@@ -644,6 +651,8 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void * + ap->entry->current = source; + ret = ctxt->parse->parse_mount(ap, name, + name_len, ent, ctxt->parse->context); ++ if (ent) ++ free(ent); + goto out_free; + } else { + if (IS_MM(me) && !IS_MM_ROOT(me)) { +diff --git a/modules/lookup_yp.c b/modules/lookup_yp.c +index 8bccb72f..d2a4b5a5 100644 +--- a/modules/lookup_yp.c ++++ b/modules/lookup_yp.c +@@ -254,7 +254,7 @@ int yp_all_master_callback(int status, char *ypkey, int ypkeylen, + + len = ypkeylen + 1 + vallen + 2; + +- buffer = alloca(len); ++ buffer = malloc(len); + if (!buffer) { + error(logopt, MODPREFIX "could not malloc parse buffer"); + return 0; +@@ -267,6 +267,8 @@ int yp_all_master_callback(int status, char *ypkey, int ypkeylen, + + master_parse_entry(buffer, timeout, logging, age); + ++ free(buffer); ++ + return 0; + } + +@@ -368,7 +370,12 @@ int yp_all_callback(int status, char *ypkey, int ypkeylen, + return 0; + } + +- mapent = alloca(vallen + 1); ++ mapent = malloc(vallen + 1); ++ if (!mapent) { ++ error(logopt, MODPREFIX "could not malloc mapent buffer"); ++ free(key); ++ return 0; ++ } + strncpy(mapent, val, vallen); + *(mapent + vallen) = '\0'; + +@@ -377,6 +384,7 @@ int yp_all_callback(int status, char *ypkey, int ypkeylen, + cache_unlock(mc); + + free(key); ++ free(mapent); + + if (ret == CHE_FAIL) + return -1; +@@ -904,7 +912,14 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void * + } + if (me && (me->source == source || *me->key == '/')) { + mapent_len = strlen(me->mapent); +- mapent = alloca(mapent_len + 1); ++ mapent = malloc(mapent_len + 1); ++ if (!mapent) { ++ char *estr = strerror_r(errno, buf, MAX_ERR_BUF); ++ error(ap->logopt, MODPREFIX "malloc: %s", estr); ++ cache_unlock(mc); ++ free(lkp_key); ++ return NSS_STATUS_TRYAGAIN; ++ } + strcpy(mapent, me->mapent); + } + } +@@ -929,6 +944,7 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void * + + ret = ctxt->parse->parse_mount(ap, key, key_len, + mapent, ctxt->parse->context); ++ free(mapent); + if (ret) { + /* Don't update negative cache when re-connecting */ + if (ap->flags & MOUNT_FLAG_REMOUNT) +diff --git a/modules/parse_sun.c b/modules/parse_sun.c +index 9190165d..c65bfce0 100644 +--- a/modules/parse_sun.c ++++ b/modules/parse_sun.c +@@ -668,9 +668,16 @@ static int sun_mount(struct autofs_point *ap, const char *root, + } + } + ++ what = malloc(loclen + 1); ++ if (!what) { ++ char buf[MAX_ERR_BUF]; ++ char *estr = strerror_r(errno, buf, MAX_ERR_BUF); ++ error(ap->logopt, MODPREFIX "malloc: %s", estr); ++ return 1; ++ } ++ + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cur_state); + if (!strcmp(fstype, "nfs") || !strcmp(fstype, "nfs4")) { +- what = alloca(loclen + 1); + memcpy(what, loc, loclen); + what[loclen] = '\0'; + +@@ -709,7 +716,6 @@ static int sun_mount(struct autofs_point *ap, const char *root, + if (!loclen) + what = NULL; + else { +- what = alloca(loclen + 1); + if (*loc == ':') { + loclen--; + memcpy(what, loc + 1, loclen); +@@ -728,6 +734,9 @@ static int sun_mount(struct autofs_point *ap, const char *root, + /* Generic mount routine */ + rv = do_mount(ap, root, name, namelen, what, fstype, options); + } ++ if (what) ++ free(what); ++ + pthread_setcancelstate(cur_state, NULL); + + if (nonstrict && rv) +diff --git a/modules/replicated.c b/modules/replicated.c +index 03d4ba1e..e68a3cc6 100644 +--- a/modules/replicated.c ++++ b/modules/replicated.c +@@ -1044,22 +1044,15 @@ done: + static int add_path(struct host *hosts, const char *path, int len) + { + struct host *this; +- char *tmp, *tmp2; +- +- tmp = alloca(len + 1); +- if (!tmp) +- return 0; +- +- strncpy(tmp, path, len); +- tmp[len] = '\0'; ++ char *tmp; + + this = hosts; + while (this) { + if (!this->path) { +- tmp2 = strdup(tmp); +- if (!tmp2) ++ tmp = strdup(path); ++ if (!tmp) + return 0; +- this->path = tmp2; ++ this->path = tmp; + } + this = this->next; + } diff --git a/autofs-5.1.7-fix-concat_options-error-handling.patch b/autofs-5.1.7-fix-concat_options-error-handling.patch new file mode 100644 index 0000000..2b170ba --- /dev/null +++ b/autofs-5.1.7-fix-concat_options-error-handling.patch @@ -0,0 +1,124 @@ +autofs-5.1.7 - fix concat_options() error handling + +From: Ian Kent + +There's a possibility of a memory leak in the mount options processing +when calling concat_options() in parse_mount() of the Sun format map +entry parsing. + +There's also a case in do_init() of the Sun map format parsing where +a previously freed value is used in a logging statement without being +set to MULL. + +So ensure concat_options() always frees it's arguments so that the +handling can be consistent in all places. + +Signed-off-by: Ian Kent +--- + CHANGELOG | 1 + + modules/parse_sun.c | 24 +++++++++++------------- + 2 files changed, 12 insertions(+), 13 deletions(-) + +diff --git a/CHANGELOG b/CHANGELOG +index ecffa933..8d050552 100644 +--- a/CHANGELOG ++++ b/CHANGELOG +@@ -78,6 +78,7 @@ + - fix direct mount deadlock. + - add missing description of null map option. + - fix nonstrict offset mount fail handling. ++- fix concat_options() error handling. + + 25/01/2021 autofs-5.1.7 + - make bind mounts propagation slave by default. +diff --git a/modules/parse_sun.c b/modules/parse_sun.c +index cdf515c6..9190165d 100644 +--- a/modules/parse_sun.c ++++ b/modules/parse_sun.c +@@ -380,7 +380,8 @@ static int do_init(int argc, const char *const *argv, struct parse_context *ctxt + if (!tmp) { + char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + logerr(MODPREFIX "concat_options: %s", estr); +- free(gbl_options); ++ /* freed in concat_options */ ++ ctxt->optstr = NULL; + } else + ctxt->optstr = tmp; + } else { +@@ -492,12 +493,16 @@ static char *concat_options(char *left, char *right) + char *ret; + + if (left == NULL || *left == '\0') { ++ if (!right || *right == '\0') ++ return NULL; + ret = strdup(right); + free(right); + return ret; + } + + if (right == NULL || *right == '\0') { ++ if (left == NULL || *left == '\0') ++ return NULL; + ret = strdup(left); + free(left); + return ret; +@@ -508,6 +513,8 @@ static char *concat_options(char *left, char *right) + if (ret == NULL) { + char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + logerr(MODPREFIX "malloc: %s", estr); ++ free(left); ++ free(right); + return NULL; + } + +@@ -989,14 +996,13 @@ static int parse_mapent(const char *ent, char *g_options, char **options, char * + if (newopt && strstr(newopt, myoptions)) { + free(myoptions); + myoptions = newopt; +- } else { ++ } else if (newopt) { + tmp = concat_options(myoptions, newopt); + if (!tmp) { + char *estr; + estr = strerror_r(errno, buf, MAX_ERR_BUF); + error(logopt, MODPREFIX + "concat_options: %s", estr); +- free(myoptions); + return 0; + } + myoptions = tmp; +@@ -1358,16 +1364,12 @@ dont_expand: + if (mnt_options && noptions && strstr(noptions, mnt_options)) { + free(mnt_options); + mnt_options = noptions; +- } else { ++ } else if (noptions) { + tmp = concat_options(mnt_options, noptions); + if (!tmp) { + char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + error(ap->logopt, + MODPREFIX "concat_options: %s", estr); +- if (noptions) +- free(noptions); +- if (mnt_options) +- free(mnt_options); + free(options); + free(pmapent); + return 1; +@@ -1387,15 +1389,11 @@ dont_expand: + if (options && mnt_options && strstr(mnt_options, options)) { + free(options); + options = mnt_options; +- } else { ++ } else if (mnt_options) { + tmp = concat_options(options, mnt_options); + if (!tmp) { + char *estr = strerror_r(errno, buf, MAX_ERR_BUF); + error(ap->logopt, MODPREFIX "concat_options: %s", estr); +- if (options) +- free(options); +- if (mnt_options) +- free(mnt_options); + free(pmapent); + return 1; + } diff --git a/autofs-5.1.7-use-default-stack-size-for-threads.patch b/autofs-5.1.7-use-default-stack-size-for-threads.patch new file mode 100644 index 0000000..09b776a --- /dev/null +++ b/autofs-5.1.7-use-default-stack-size-for-threads.patch @@ -0,0 +1,128 @@ +autofs-5.1.7 - use default stack size for threads + +From: Ian Kent + +autofs uses PTHREAD_STACK_MIN to set the stack size for threads it +creates. + +In two cases it is used to reduce the stack size for long running +service threads while it's used to allocate a larger stack for worker +threads that can have larger memory requirements. + +In recent glibc releases PTHREAD_STACK_MIN is no longer a constant +which can lead to unexpectedly different stack sizes on different +architectures and the autofs assumption it's a constant causes a +compile failure. + +The need to alter the stack size was due to observed stack overflow +which was thought to be due the thread stack being too small for autofs +and glibc alloca(3) usage. + +Quite a bit of that alloca(3) usage has been eliminated from autofs now, +particularly those that might be allocating largish amounts of storage, +and there has been a lot of change in glibc too so using the thread +default stack should be ok. + +Signed-off-by: Ian Kent +--- + CHANGELOG | 1 + + daemon/automount.c | 29 ----------------------------- + daemon/state.c | 6 +----- + lib/alarm.c | 6 +----- + 4 files changed, 3 insertions(+), 39 deletions(-) + +diff --git a/CHANGELOG b/CHANGELOG +index 2b7cfaa0..61f3547a 100644 +--- a/CHANGELOG ++++ b/CHANGELOG +@@ -80,6 +80,7 @@ + - fix nonstrict offset mount fail handling. + - fix concat_options() error handling. + - eliminate some more alloca usage. ++- use default stack size for threads. + + 25/01/2021 autofs-5.1.7 + - make bind mounts propagation slave by default. +diff --git a/daemon/automount.c b/daemon/automount.c +index 23235a7d..d7432350 100644 +--- a/daemon/automount.c ++++ b/daemon/automount.c +@@ -84,7 +84,6 @@ static size_t kpkt_len; + /* Attributes for creating detached and joinable threads */ + pthread_attr_t th_attr; + pthread_attr_t th_attr_detached; +-size_t detached_thread_stack_size = PTHREAD_STACK_MIN * 144; + + struct master_readmap_cond mrc = { + PTHREAD_MUTEX_INITIALIZER, PTHREAD_COND_INITIALIZER, 0, NULL, 0, 0, 0, 0}; +@@ -2620,34 +2619,6 @@ int main(int argc, char *argv[]) + exit(1); + } + +-#ifdef _POSIX_THREAD_ATTR_STACKSIZE +- if (pthread_attr_setstacksize( +- &th_attr_detached, detached_thread_stack_size)) { +- logerr("%s: failed to set stack size thread attribute!", +- program); +- if (start_pipefd[1] != -1) { +- res = write(start_pipefd[1], pst_stat, sizeof(*pst_stat)); +- close(start_pipefd[1]); +- } +- release_flag_file(); +- macro_free_global_table(); +- exit(1); +- } +-#endif +- +- if (pthread_attr_getstacksize( +- &th_attr_detached, &detached_thread_stack_size)) { +- logerr("%s: failed to get detached thread stack size!", +- program); +- if (start_pipefd[1] != -1) { +- res = write(start_pipefd[1], pst_stat, sizeof(*pst_stat)); +- close(start_pipefd[1]); +- } +- release_flag_file(); +- macro_free_global_table(); +- exit(1); +- } +- + info(logging, "Starting automounter version %s, master map %s", + version, master_list->name); + info(logging, "using kernel protocol version %d.%02d", +diff --git a/daemon/state.c b/daemon/state.c +index 5156bb21..5df05619 100644 +--- a/daemon/state.c ++++ b/daemon/state.c +@@ -1177,12 +1177,8 @@ int st_start_handler(void) + status = pthread_attr_init(pattrs); + if (status) + pattrs = NULL; +- else { ++ else + pthread_attr_setdetachstate(pattrs, PTHREAD_CREATE_DETACHED); +-#ifdef _POSIX_THREAD_ATTR_STACKSIZE +- pthread_attr_setstacksize(pattrs, PTHREAD_STACK_MIN*4); +-#endif +- } + + status = pthread_create(&thid, pattrs, st_queue_handler, NULL); + +diff --git a/lib/alarm.c b/lib/alarm.c +index f27e13c4..1631a9bc 100755 +--- a/lib/alarm.c ++++ b/lib/alarm.c +@@ -270,12 +270,8 @@ int alarm_start_handler(void) + status = pthread_attr_init(pattrs); + if (status) + pattrs = NULL; +- else { ++ else + pthread_attr_setdetachstate(pattrs, PTHREAD_CREATE_DETACHED); +-#ifdef _POSIX_THREAD_ATTR_STACKSIZE +- pthread_attr_setstacksize(pattrs, PTHREAD_STACK_MIN*4); +-#endif +- } + + status = pthread_condattr_init(&condattrs); + if (status) diff --git a/autofs.spec b/autofs.spec index 181806f..a6e692a 100644 --- a/autofs.spec +++ b/autofs.spec @@ -12,7 +12,7 @@ Summary: A tool for automatically mounting and unmounting filesystems Name: autofs Version: 5.1.7 -Release: 18%{?dist} +Release: 19%{?dist} Epoch: 1 License: GPLv2+ Source: https://www.kernel.org/pub/linux/daemons/autofs/v5/autofs-%{version}.tar.gz @@ -96,6 +96,9 @@ Patch76: autofs-5.1.7-fix-hosts-map-offset-order.patch Patch77: autofs-5.1.7-fix-direct-mount-deadlock.patch Patch78: autofs-5.1.7-add-missing-description-of-null-map-option.patch Patch79: autofs-5.1.7-fix-nonstrict-offset-mount-fail-handling.patch +Patch80: autofs-5.1.7-fix-concat_options-error-handling.patch +Patch81: autofs-5.1.7-eliminate-some-more-alloca-usage.patch +Patch82: autofs-5.1.7-use-default-stack-size-for-threads.patch %if %{with_systemd} BuildRequires: systemd-units @@ -239,6 +242,9 @@ echo %{version}-%{release} > .version %patch77 -p1 %patch78 -p1 %patch79 -p1 +%patch80 -p1 +%patch81 -p1 +%patch82 -p1 %build LDFLAGS=-Wl,-z,now @@ -347,6 +353,11 @@ fi %dir /etc/auto.master.d %changelog +* Sat Jun 19 2021 Ian Kent - 1:5.1.7-19 +- fix concat_options() error handling. +- eliminate some more alloca usage. +- use default stack size for threads. + * Wed Jul 21 2021 Fedora Release Engineering - 1:5.1.7-18 - Rebuilt for https://fedoraproject.org/wiki/Fedora_35_Mass_Rebuild