From: Todd C. Miller Date: Fri, 11 Nov 2016 23:18:27 +0000 (-0700) Subject: Add simple reference-counted string allocator and use it for passing X-Git-Tag: SUDO_1_8_19^2~48 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=90995c0acf95e33381cd91fea5d2bac46f60d22c;p=sudo Add simple reference-counted string allocator and use it for passing around references to the sudoers path. This lets us avoid making copies of the sudoers path for the errorfile as well as each Defaults entry. --- diff --git a/MANIFEST b/MANIFEST index 2803d1a30..564a1e4c2 100644 --- a/MANIFEST +++ b/MANIFEST @@ -357,6 +357,7 @@ plugins/sudoers/prompt.c plugins/sudoers/pwutil.c plugins/sudoers/pwutil.h plugins/sudoers/pwutil_impl.c +plugins/sudoers/rcstr.c plugins/sudoers/redblack.c plugins/sudoers/redblack.h plugins/sudoers/regress/check_symbols/check_symbols.c diff --git a/plugins/sudoers/Makefile.in b/plugins/sudoers/Makefile.in index 2a035a5b5..1ee537faa 100644 --- a/plugins/sudoers/Makefile.in +++ b/plugins/sudoers/Makefile.in @@ -148,8 +148,8 @@ AUTH_OBJS = sudo_auth.lo @AUTH_OBJS@ LIBPARSESUDOERS_OBJS = alias.lo audit.lo base64.lo defaults.lo hexchar.lo \ gram.lo match.lo match_addr.lo pwutil.lo pwutil_impl.lo \ - redblack.lo sudoers_debug.lo timestr.lo toke.lo \ - toke_util.lo + rcstr.lo redblack.lo sudoers_debug.lo timestr.lo \ + toke.lo toke_util.lo SUDOERS_OBJS = $(AUTH_OBJS) boottime.lo check.lo editor.lo env.lo find_path.lo \ gc.lo goodpath.lo group_plugin.lo interfaces.lo iolog.lo \ @@ -918,6 +918,14 @@ pwutil_impl.lo: $(srcdir)/pwutil_impl.c $(devdir)/def_data.h \ $(top_builddir)/pathnames.h $(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) -c $(CPPFLAGS) $(CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/pwutil_impl.c pwutil_impl.o: pwutil_impl.lo +rcstr.lo: $(srcdir)/rcstr.c $(devdir)/def_data.h $(incdir)/compat/stdbool.h \ + $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ + $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \ + $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \ + $(srcdir)/defaults.h $(srcdir)/logging.h $(srcdir)/sudo_nss.h \ + $(srcdir)/sudoers.h $(srcdir)/sudoers_debug.h \ + $(top_builddir)/config.h $(top_builddir)/pathnames.h + $(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) -c $(CPPFLAGS) $(CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/rcstr.c redblack.lo: $(srcdir)/redblack.c $(devdir)/def_data.h \ $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \ $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h \ diff --git a/plugins/sudoers/gram.c b/plugins/sudoers/gram.c index ef2b279c1..896cb0b0d 100644 --- a/plugins/sudoers/gram.c +++ b/plugins/sudoers/gram.c @@ -702,11 +702,8 @@ sudoerserror(const char *s) /* Save the line the first error occurred on. */ if (errorlineno == -1) { errorlineno = sudolineno; - free(errorfile); - errorfile = strdup(sudoers); - if (errorfile == NULL) - sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, - "unable to allocate memory"); + rcstr_delref(errorfile); + errorfile = rcstr_addref(sudoers); } if (sudoers_warnings && s != NULL) { LEXTRACE("<*> "); @@ -744,13 +741,7 @@ new_default(char *var, char *val, short op) d->op = op; /* d->binding = NULL */ d->lineno = last_token == COMMENT ? sudolineno - 1 : sudolineno; - d->file = strdup(sudoers); - if (d->file == NULL) { - sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, - "unable to allocate memory"); - free(d); - debug_return_ptr(NULL); - } + d->file = rcstr_addref(sudoers); HLTQ_INIT(d, entries); debug_return_ptr(d); @@ -981,10 +972,9 @@ init_parser(const char *path, bool quiet) free_members(d->binding); free(d->binding); } - /* no need to free sd_un */ + rcstr_delref(d->file); free(d->var); free(d->val); - free(d->file); free(d); } TAILQ_INIT(&defaults); @@ -996,9 +986,9 @@ init_parser(const char *path, bool quiet) ret = false; } - free(sudoers); + rcstr_delref(sudoers); if (path != NULL) { - if ((sudoers = strdup(path)) == NULL) { + if ((sudoers = rcstr_dup(path)) == NULL) { sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); ret = false; } @@ -1008,13 +998,13 @@ init_parser(const char *path, bool quiet) parse_error = false; errorlineno = -1; - free(errorfile); + rcstr_delref(errorfile); errorfile = NULL; sudoers_warnings = !quiet; debug_return_bool(ret); } -#line 965 "gram.c" +#line 955 "gram.c" /* allocate initial stack or double stack size, up to YYMAXDEPTH */ #if defined(__cplusplus) || defined(__STDC__) static int yygrowstack(void) @@ -2097,7 +2087,7 @@ case 115: } } break; -#line 2048 "gram.c" +#line 2038 "gram.c" } yyssp -= yym; yystate = *yyssp; diff --git a/plugins/sudoers/gram.y b/plugins/sudoers/gram.y index 70e24f78e..c58aeeb09 100644 --- a/plugins/sudoers/gram.y +++ b/plugins/sudoers/gram.y @@ -858,11 +858,8 @@ sudoerserror(const char *s) /* Save the line the first error occurred on. */ if (errorlineno == -1) { errorlineno = sudolineno; - free(errorfile); - errorfile = strdup(sudoers); - if (errorfile == NULL) - sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, - "unable to allocate memory"); + rcstr_delref(errorfile); + errorfile = rcstr_addref(sudoers); } if (sudoers_warnings && s != NULL) { LEXTRACE("<*> "); @@ -900,13 +897,7 @@ new_default(char *var, char *val, short op) d->op = op; /* d->binding = NULL */ d->lineno = last_token == COMMENT ? sudolineno - 1 : sudolineno; - d->file = strdup(sudoers); - if (d->file == NULL) { - sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO, - "unable to allocate memory"); - free(d); - debug_return_ptr(NULL); - } + d->file = rcstr_addref(sudoers); HLTQ_INIT(d, entries); debug_return_ptr(d); @@ -1137,10 +1128,9 @@ init_parser(const char *path, bool quiet) free_members(d->binding); free(d->binding); } - /* no need to free sd_un */ + rcstr_delref(d->file); free(d->var); free(d->val); - free(d->file); free(d); } TAILQ_INIT(&defaults); @@ -1152,9 +1142,9 @@ init_parser(const char *path, bool quiet) ret = false; } - free(sudoers); + rcstr_delref(sudoers); if (path != NULL) { - if ((sudoers = strdup(path)) == NULL) { + if ((sudoers = rcstr_dup(path)) == NULL) { sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); ret = false; } @@ -1164,7 +1154,7 @@ init_parser(const char *path, bool quiet) parse_error = false; errorlineno = -1; - free(errorfile); + rcstr_delref(errorfile); errorfile = NULL; sudoers_warnings = !quiet; diff --git a/plugins/sudoers/rcstr.c b/plugins/sudoers/rcstr.c new file mode 100644 index 000000000..e3f461bfa --- /dev/null +++ b/plugins/sudoers/rcstr.c @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2016 Todd C. Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include + +#include + +#include +#include +#include +#ifdef HAVE_STRING_H +# include +#endif /* HAVE_STRING_H */ +#ifdef HAVE_STRINGS_H +# include +#endif /* HAVE_STRINGS_H */ + +#include "sudoers.h" + +/* Trivial reference-counted strings. */ +struct rcstr { + int refcnt; + char str[1]; /* actually bigger */ +}; + +/* + * Allocate a reference-counted string and copy src to it. + * Returns the newly-created string with a refcnt of 1. + */ +char * +rcstr_dup(const char *src) +{ + size_t len = strlen(src); + char *dst; + debug_decl(rcstr_dup, SUDOERS_DEBUG_UTIL) + + dst = rcstr_alloc(len); + memcpy(dst, src, len); + dst[len] = '\0'; + debug_return_ptr(dst); +} + +char * +rcstr_alloc(size_t len) +{ + struct rcstr *rcs; + debug_decl(rcstr_dup, SUDOERS_DEBUG_UTIL) + + /* Note: sizeof(struct rcstr) includes space for the NUL */ + rcs = malloc(sizeof(struct rcstr) + len); + if (rcs == NULL) + return NULL; + + rcs->refcnt = 1; + rcs->str[0] = '\0'; + debug_return_ptr(rcs->str); +} + +char * +rcstr_addref(const char *s) +{ + struct rcstr *rcs; + debug_decl(rcstr_dup, SUDOERS_DEBUG_UTIL) + + if (s == NULL) + debug_return_ptr(NULL); + + rcs = __containerof(s, struct rcstr, str); + rcs->refcnt++; + debug_return_ptr(rcs->str); +} + +void +rcstr_delref(const char *s) +{ + struct rcstr *rcs; + debug_decl(rcstr_dup, SUDOERS_DEBUG_UTIL) + + if (s != NULL) { + rcs = __containerof(s, struct rcstr, str); + if (--rcs->refcnt == 0) { + rcs->str[0] = '\0'; + free(rcs); + } + } + debug_return; +} diff --git a/plugins/sudoers/sudoers.h b/plugins/sudoers/sudoers.h index 65952a544..1971ac82a 100644 --- a/plugins/sudoers/sudoers.h +++ b/plugins/sudoers/sudoers.h @@ -403,4 +403,10 @@ bool sudoers_gc_add(enum sudoers_gc_types type, void *ptr); bool sudoers_gc_remove(enum sudoers_gc_types type, void *ptr); void sudoers_gc_init(void); +/* rcstr.c */ +char *rcstr_dup(const char *src); +char *rcstr_alloc(size_t len); +char *rcstr_addref(const char *s); +void rcstr_delref(const char *s); + #endif /* SUDOERS_SUDOERS_H */ diff --git a/plugins/sudoers/toke.c b/plugins/sudoers/toke.c index c23da7c9a..e5b4d9750 100644 --- a/plugins/sudoers/toke.c +++ b/plugins/sudoers/toke.c @@ -4068,6 +4068,7 @@ read_dir_files(const char *dirpath, struct path_list ***pathsp) while ((dent = readdir(dir)) != NULL) { struct path_list *pl; struct stat sb; + size_t len; char *path; /* Ignore files that end in '~' or have a '.' in them. */ @@ -4075,15 +4076,17 @@ read_dir_files(const char *dirpath, struct path_list ***pathsp) || strchr(dent->d_name, '.') != NULL) { continue; } - if (asprintf(&path, "%s/%s", dirpath, dent->d_name) == -1) + len = strlen(dirpath) + 1 + NAMLEN(dent); + if ((path = rcstr_alloc(len)) == NULL) goto oom; + (void)snprintf(path, len + 1, "%s/%s", dirpath, dent->d_name); if (stat(path, &sb) != 0 || !S_ISREG(sb.st_mode)) { - free(path); + rcstr_delref(path); continue; } pl = malloc(sizeof(*pl)); if (pl == NULL) { - free(path); + rcstr_delref(path); goto oom; } pl->path = path; @@ -4092,7 +4095,7 @@ read_dir_files(const char *dirpath, struct path_list ***pathsp) max_paths <<= 1; tmp = reallocarray(paths, max_paths, sizeof(*paths)); if (tmp == NULL) { - free(path); + rcstr_delref(path); free(pl); goto oom; } @@ -4115,7 +4118,7 @@ bad: if (dir != NULL) closedir(dir); for (i = 0; i < count; i++) { - free(paths[i]->path); + rcstr_delref(paths[i]->path); free(paths[i]); } free(paths); @@ -4165,10 +4168,10 @@ init_lexer(void) idepth--; while ((pl = SLIST_FIRST(&istack[idepth].more)) != NULL) { SLIST_REMOVE_HEAD(&istack[idepth].more, entries); - free(pl->path); + rcstr_delref(pl->path); free(pl); } - free(istack[idepth].path); + rcstr_delref(istack[idepth].path); if (idepth && !istack[idepth].keepopen) fclose(istack[idepth].bs->yy_input_file); sudoers_delete_buffer(istack[idepth].bs); @@ -4251,13 +4254,13 @@ push_include_int(char *path, bool isdir) count = switch_dir(&istack[idepth], path); if (count <= 0) { /* switch_dir() called sudoerserror() for us */ - free(path); + rcstr_delref(path); debug_return_bool(count ? false : true); } /* Parse the first dir entry we can open, leave the rest for later. */ do { - free(path); + rcstr_delref(path); if ((pl = SLIST_FIRST(&istack[idepth].more)) == NULL) { /* Unable to open any files in include dir, not an error. */ debug_return_bool(true); @@ -4274,7 +4277,7 @@ push_include_int(char *path, bool isdir) } } /* Push the old (current) file and open the new one. */ - istack[idepth].path = sudoers; /* push old path */ + istack[idepth].path = sudoers; /* push old path (and its ref) */ istack[idepth].bs = YY_CURRENT_BUFFER; istack[idepth].lineno = sudolineno; istack[idepth].keepopen = keepopen; @@ -4309,7 +4312,7 @@ pop_include(void) SLIST_REMOVE_HEAD(&istack[idepth - 1].more, entries); fp = open_sudoers(pl->path, false, &keepopen); if (fp != NULL) { - free(sudoers); + rcstr_delref(sudoers); sudoers = pl->path; sudolineno = 1; sudoers_switch_to_buffer(sudoers_create_buffer(fp, YY_BUF_SIZE)); @@ -4317,14 +4320,14 @@ pop_include(void) break; } /* Unable to open path in include dir, go to next one. */ - free(pl->path); + rcstr_delref(pl->path); free(pl); } /* If no path list, just pop the last dir on the stack. */ if (pl == NULL) { idepth--; sudoers_switch_to_buffer(istack[idepth].bs); - free(sudoers); + rcstr_delref(sudoers); sudoers = istack[idepth].path; sudolineno = istack[idepth].lineno; keepopen = istack[idepth].keepopen; @@ -4365,7 +4368,7 @@ parse_include(char *base) /* Make a copy of the fully-qualified path and return it. */ len += (int)(ep - cp); - path = pp = malloc(len + dirlen + 1); + path = pp = rcstr_alloc(len + dirlen); if (path == NULL) { sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); sudoerserror(NULL); diff --git a/plugins/sudoers/toke.l b/plugins/sudoers/toke.l index a98fef0dc..b63edd073 100644 --- a/plugins/sudoers/toke.l +++ b/plugins/sudoers/toke.l @@ -785,6 +785,7 @@ read_dir_files(const char *dirpath, struct path_list ***pathsp) while ((dent = readdir(dir)) != NULL) { struct path_list *pl; struct stat sb; + size_t len; char *path; /* Ignore files that end in '~' or have a '.' in them. */ @@ -792,15 +793,17 @@ read_dir_files(const char *dirpath, struct path_list ***pathsp) || strchr(dent->d_name, '.') != NULL) { continue; } - if (asprintf(&path, "%s/%s", dirpath, dent->d_name) == -1) + len = strlen(dirpath) + 1 + NAMLEN(dent); + if ((path = rcstr_alloc(len)) == NULL) goto oom; + (void)snprintf(path, len + 1, "%s/%s", dirpath, dent->d_name); if (stat(path, &sb) != 0 || !S_ISREG(sb.st_mode)) { - free(path); + rcstr_delref(path); continue; } pl = malloc(sizeof(*pl)); if (pl == NULL) { - free(path); + rcstr_delref(path); goto oom; } pl->path = path; @@ -809,7 +812,7 @@ read_dir_files(const char *dirpath, struct path_list ***pathsp) max_paths <<= 1; tmp = reallocarray(paths, max_paths, sizeof(*paths)); if (tmp == NULL) { - free(path); + rcstr_delref(path); free(pl); goto oom; } @@ -832,7 +835,7 @@ bad: if (dir != NULL) closedir(dir); for (i = 0; i < count; i++) { - free(paths[i]->path); + rcstr_delref(paths[i]->path); free(paths[i]); } free(paths); @@ -882,10 +885,10 @@ init_lexer(void) idepth--; while ((pl = SLIST_FIRST(&istack[idepth].more)) != NULL) { SLIST_REMOVE_HEAD(&istack[idepth].more, entries); - free(pl->path); + rcstr_delref(pl->path); free(pl); } - free(istack[idepth].path); + rcstr_delref(istack[idepth].path); if (idepth && !istack[idepth].keepopen) fclose(istack[idepth].bs->yy_input_file); sudoers_delete_buffer(istack[idepth].bs); @@ -968,13 +971,13 @@ push_include_int(char *path, bool isdir) count = switch_dir(&istack[idepth], path); if (count <= 0) { /* switch_dir() called sudoerserror() for us */ - free(path); + rcstr_delref(path); debug_return_bool(count ? false : true); } /* Parse the first dir entry we can open, leave the rest for later. */ do { - free(path); + rcstr_delref(path); if ((pl = SLIST_FIRST(&istack[idepth].more)) == NULL) { /* Unable to open any files in include dir, not an error. */ debug_return_bool(true); @@ -991,7 +994,7 @@ push_include_int(char *path, bool isdir) } } /* Push the old (current) file and open the new one. */ - istack[idepth].path = sudoers; /* push old path */ + istack[idepth].path = sudoers; /* push old path (and its ref) */ istack[idepth].bs = YY_CURRENT_BUFFER; istack[idepth].lineno = sudolineno; istack[idepth].keepopen = keepopen; @@ -1026,7 +1029,7 @@ pop_include(void) SLIST_REMOVE_HEAD(&istack[idepth - 1].more, entries); fp = open_sudoers(pl->path, false, &keepopen); if (fp != NULL) { - free(sudoers); + rcstr_delref(sudoers); sudoers = pl->path; sudolineno = 1; sudoers_switch_to_buffer(sudoers_create_buffer(fp, YY_BUF_SIZE)); @@ -1034,14 +1037,14 @@ pop_include(void) break; } /* Unable to open path in include dir, go to next one. */ - free(pl->path); + rcstr_delref(pl->path); free(pl); } /* If no path list, just pop the last dir on the stack. */ if (pl == NULL) { idepth--; sudoers_switch_to_buffer(istack[idepth].bs); - free(sudoers); + rcstr_delref(sudoers); sudoers = istack[idepth].path; sudolineno = istack[idepth].lineno; keepopen = istack[idepth].keepopen; @@ -1082,7 +1085,7 @@ parse_include(char *base) /* Make a copy of the fully-qualified path and return it. */ len += (int)(ep - cp); - path = pp = malloc(len + dirlen + 1); + path = pp = rcstr_alloc(len + dirlen); if (path == NULL) { sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory")); sudoerserror(NULL); diff --git a/plugins/sudoers/visudo.c b/plugins/sudoers/visudo.c index f33c1db34..c7bf710fd 100644 --- a/plugins/sudoers/visudo.c +++ b/plugins/sudoers/visudo.c @@ -562,22 +562,20 @@ check_defaults_and_aliases(bool strict, bool quiet) if (!check_defaults(quiet)) { struct defaults *d; - free(errorfile); + rcstr_delref(errorfile); errorfile = NULL; /* XXX - should edit all files with errors */ TAILQ_FOREACH(d, &defaults, entries) { if (d->error) { - /* Defaults parse error, adopt the file name. */ - errorfile = d->file; + /* Defaults parse error, set errorfile/errorlineno. */ + errorfile = rcstr_addref(d->file); errorlineno = d->lineno; - d->file = NULL; - d->error = false; /* paranoia */ break; } } parse_error = true; } else if (check_aliases(strict, quiet) != 0) { - free(errorfile); + rcstr_delref(errorfile); errorfile = NULL; /* don't know which file */ parse_error = true; } @@ -618,8 +616,8 @@ reparse_sudoers(char *editor, int editor_argc, char **editor_argv, sudo_warnx(U_("unabled to parse temporary file (%s), unknown error"), sp->tpath); parse_error = true; - free(errorfile); - if ((errorfile = strdup(sp->path)) == NULL) + rcstr_delref(errorfile); + if ((errorfile = rcstr_dup(sp->path)) == NULL) sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory")); } fclose(sudoersin); @@ -959,8 +957,8 @@ check_syntax(const char *sudoers_file, bool quiet, bool strict, bool oldperms) if (!quiet) sudo_warnx(U_("failed to parse %s file, unknown error"), sudoers_file); parse_error = true; - free(errorfile); - if ((errorfile = strdup(sudoers_file)) == NULL) + rcstr_delref(errorfile); + if ((errorfile = rcstr_dup(sudoers_file)) == NULL) sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory")); } if (!parse_error) { diff --git a/plugins/sudoers/visudo_json.c b/plugins/sudoers/visudo_json.c index dd2ced273..d21bf90f5 100644 --- a/plugins/sudoers/visudo_json.c +++ b/plugins/sudoers/visudo_json.c @@ -1030,8 +1030,8 @@ export_sudoers(const char *sudoers_path, const char *export_path, if (!quiet) sudo_warnx(U_("failed to parse %s file, unknown error"), sudoers_path); parse_error = true; - free(errorfile); - if ((errorfile = strdup(sudoers_path)) == NULL) + rcstr_delref(errorfile); + if ((errorfile = rcstr_dup(sudoers_path)) == NULL) sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory")); } ret = !parse_error;