From 335b63d8d1876ce4e172ebcc9d64544785682244 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Tue, 10 Apr 2007 21:38:30 +0000 Subject: [PATCH] make a few struct bb_applet members conditional rename sllep_and_die -> xfunc_die make fflush_stdout_and_exit NOFORK-safe fix some buglets found by randomconfig --- applets/applets.c | 53 ++++++++++++++++------------------ archival/libunarchive/Kbuild | 5 +++- coreutils/cmp.c | 4 +-- coreutils/echo.c | 2 ++ include/applets.h | 14 ++++----- include/busybox.h | 26 ++++++++++++----- include/libbb.h | 10 +++---- libbb/copyfd.c | 4 +-- libbb/error_msg_and_die.c | 21 ++++++++++---- libbb/fflush_stdout_and_exit.c | 14 +++++---- libbb/herror_msg_and_die.c | 5 +--- libbb/perror_msg_and_die.c | 6 +--- libbb/vfork_daemon_rexec.c | 23 +++++++++++---- libbb/xconnect.c | 2 +- libbb/xfuncs.c | 2 +- shell/ash.c | 2 +- shell/msh.c | 2 +- util-linux/getopt.c | 2 -- 18 files changed, 114 insertions(+), 83 deletions(-) diff --git a/applets/applets.c b/applets/applets.c index 66f2b821d..3441a7886 100644 --- a/applets/applets.c +++ b/applets/applets.c @@ -43,13 +43,13 @@ static const char usage_messages[] = #define usage_messages 0 #endif /* SHOW_USAGE */ -/* Define struct BB_applet applets[] */ +/* Define struct bb_applet applets[] */ #include "applets.h" /* The -1 arises because of the {0,NULL,0,-1} entry. */ -const unsigned short NUM_APPLETS = sizeof(applets) / sizeof(struct BB_applet) - 1; +const unsigned short NUM_APPLETS = sizeof(applets) / sizeof(applets[0]) - 1; -const struct BB_applet *current_applet; +const struct bb_applet *current_applet; const char *applet_name ATTRIBUTE_EXTERNALLY_VISIBLE; #ifdef BB_NOMMU bool re_execed; @@ -61,7 +61,7 @@ bool re_execed; /* applets[] is const, so we have to define this "override" structure */ static struct BB_suid_config { - const struct BB_applet *m_applet; + const struct bb_applet *m_applet; uid_t m_uid; gid_t m_gid; mode_t m_mode; @@ -130,7 +130,7 @@ static void parse_config_file(void) { struct BB_suid_config *sct_head; struct BB_suid_config *sct; - const struct BB_applet *applet; + const struct bb_applet *applet; FILE *f; const char *errmsg; char *s; @@ -329,7 +329,7 @@ static void parse_config_file(void) #if ENABLE_FEATURE_SUID -static void check_suid(const struct BB_applet *applet) +static void check_suid(const struct bb_applet *applet) { uid_t ruid = getuid(); /* real [ug]id */ uid_t rgid = getgid(); @@ -464,45 +464,42 @@ void bb_show_usage(void) applet_name, usage_string); } - sleep_and_die(); + xfunc_die(); } static int applet_name_compare(const void *name, const void *vapplet) { - const struct BB_applet *applet = vapplet; + const struct bb_applet *applet = vapplet; return strcmp(name, applet->name); } -const struct BB_applet *find_applet_by_name(const char *name) +const struct bb_applet *find_applet_by_name(const char *name) { /* Do a binary search to find the applet entry given the name. */ - return bsearch(name, applets, NUM_APPLETS, sizeof(struct BB_applet), + return bsearch(name, applets, NUM_APPLETS, sizeof(applets[0]), applet_name_compare); } #if ENABLE_FEATURE_INSTALLER -/* - * directory table - * this should be consistent w/ the enum, busybox.h::Location, - * or else... - */ -static const char usr_bin [] = "/usr/bin"; -static const char usr_sbin[] = "/usr/sbin"; - -static const char *const install_dir[] = { - &usr_bin [8], /* "", equivalent to "/" for concat_path_file() */ - &usr_bin [4], /* "/bin" */ - &usr_sbin[4], /* "/sbin" */ - usr_bin, - usr_sbin -}; - /* create (sym)links for each applet */ static void install_links(const char *busybox, int use_symbolic_links) { + /* directory table + * this should be consistent w/ the enum, + * busybox.h::bb_install_loc_t, or else... */ + static const char usr_bin [] = "/usr/bin"; + static const char usr_sbin[] = "/usr/sbin"; + static const char *const install_dir[] = { + &usr_bin [8], /* "", equivalent to "/" for concat_path_file() */ + &usr_bin [4], /* "/bin" */ + &usr_sbin[4], /* "/sbin" */ + usr_bin, + usr_sbin + }; + int (*lf)(const char *, const char *) = link; char *fpc; int i; @@ -513,7 +510,7 @@ static void install_links(const char *busybox, int use_symbolic_links) for (i = 0; applets[i].name != NULL; i++) { fpc = concat_path_file( - install_dir[applets[i].location], + install_dir[applets[i].install_loc], applets[i].name); rc = lf(busybox, fpc); if (rc != 0 && errno != EEXIST) { @@ -557,7 +554,7 @@ static int busybox_main(int argc, char **argv) applet_name = argv[2]; run_applet_by_name(applet_name, 2, argv); } else { - const struct BB_applet *a; + const struct bb_applet *a; int col, output_width; output_width = 80 - sizeof("start-stop-daemon, ") - 8; diff --git a/archival/libunarchive/Kbuild b/archival/libunarchive/Kbuild index 010043c4c..412a2332d 100644 --- a/archival/libunarchive/Kbuild +++ b/archival/libunarchive/Kbuild @@ -35,12 +35,15 @@ DPKG_FILES:= \ get_header_tar.o \ filter_accept_list_reassign.o -# open_transformer uses fork. Compile it only if absolutely necessary +# open_transformer uses fork(). Compile it only if absolutely necessary lib-$(CONFIG_RPM) += open_transformer.o lib-$(CONFIG_FEATURE_TAR_BZIP2) += open_transformer.o lib-$(CONFIG_FEATURE_TAR_LZMA) += open_transformer.o lib-$(CONFIG_FEATURE_TAR_GZIP) += open_transformer.o lib-$(CONFIG_FEATURE_TAR_COMPRESS) += open_transformer.o +lib-$(CONFIG_FEATURE_DEB_TAR_GZ) += open_transformer.o +lib-$(CONFIG_FEATURE_DEB_TAR_BZ2) += open_transformer.o +lib-$(CONFIG_FEATURE_DEB_TAR_LZMA) += open_transformer.o lib-$(CONFIG_AR) += get_header_ar.o unpack_ar_archive.o lib-$(CONFIG_BUNZIP2) += decompress_bunzip2.o diff --git a/coreutils/cmp.c b/coreutils/cmp.c index c70f8822d..80fab0b90 100644 --- a/coreutils/cmp.c +++ b/coreutils/cmp.c @@ -23,14 +23,14 @@ #include "busybox.h" -static FILE *cmp_xfopen_input(const char * const filename) +static FILE *cmp_xfopen_input(const char *filename) { FILE *fp; fp = fopen_or_warn_stdin(filename); if (fp) return fp; - sleep_and_die(); /* We already output an error message. */ + xfunc_die(); /* We already output an error message. */ } static const char fmt_eof[] = "cmp: EOF on %s\n"; diff --git a/coreutils/echo.c b/coreutils/echo.c index 2ee5002ba..2de19c2e3 100644 --- a/coreutils/echo.c +++ b/coreutils/echo.c @@ -107,7 +107,9 @@ int bb_echo(char **argv) putchar(' '); } +#if ENABLE_FEATURE_FANCY_ECHO newline_ret: +#endif if (nflag) { putchar('\n'); } diff --git a/include/applets.h b/include/applets.h index b59d33183..d05299b69 100644 --- a/include/applets.h +++ b/include/applets.h @@ -52,12 +52,12 @@ s - suid type: # define APPLET_NOFORK(name,main,l,s,name2) LINK l name #else - const struct BB_applet applets[] = { /* name,main,location,need_suid */ -# define APPLET(name,l,s) {#name,name##_main,l,s}, -# define APPLET_NOUSAGE(name,main,l,s) {#name,main##_main,l,s}, -# define APPLET_ODDNAME(name,main,l,s,name2) {#name,main##_main,l,s}, -# define APPLET_NOEXEC(name,main,l,s,name2) {#name,main##_main,l,s,1}, -# define APPLET_NOFORK(name,main,l,s,name2) {#name,main##_main,l,s,1,1}, + const struct bb_applet applets[] = { /* name, main, location, need_suid */ +# define APPLET(name,l,s) { #name, name##_main USE_FEATURE_INSTALLER(,l) USE_FEATURE_SUID(,s) }, +# define APPLET_NOUSAGE(name,main,l,s) { #name, main##_main USE_FEATURE_INSTALLER(,l) USE_FEATURE_SUID(,s) }, +# define APPLET_ODDNAME(name,main,l,s,name2) { #name, main##_main USE_FEATURE_INSTALLER(,l) USE_FEATURE_SUID(,s) }, +# define APPLET_NOEXEC(name,main,l,s,name2) { #name, main##_main USE_FEATURE_INSTALLER(,l) USE_FEATURE_SUID(,s) USE_FEATURE_EXEC_PREFER_APPLETS(,1) }, +# define APPLET_NOFORK(name,main,l,s,name2) { #name, main##_main USE_FEATURE_INSTALLER(,l) USE_FEATURE_SUID(,s) USE_FEATURE_EXEC_PREFER_APPLETS(,1 ,1) }, #endif #if ENABLE_INSTALL_NO_USR @@ -355,7 +355,7 @@ USE_GUNZIP(APPLET_ODDNAME(zcat, gunzip, _BB_DIR_BIN, _BB_SUID_NEVER, zcat)) USE_ZCIP(APPLET(zcip, _BB_DIR_SBIN, _BB_SUID_NEVER)) #if !defined(PROTOTYPES) && !defined(MAKE_USAGE) - { 0, NULL, 0, 0 } + { NULL } }; #endif diff --git a/include/busybox.h b/include/busybox.h index 6f4808778..380de9ab8 100644 --- a/include/busybox.h +++ b/include/busybox.h @@ -9,26 +9,35 @@ #include "libbb.h" +#if ENABLE_FEATURE_INSTALLER /* order matters: used as index into "install_dir[]" in busybox.c */ -enum Location { +typedef enum bb_install_loc_t { _BB_DIR_ROOT = 0, _BB_DIR_BIN, _BB_DIR_SBIN, _BB_DIR_USR_BIN, _BB_DIR_USR_SBIN -}; +} bb_install_loc_t; +#endif -enum SUIDRoot { +#if ENABLE_FEATURE_SUID +typedef enum bb_suid_t { _BB_SUID_NEVER = 0, _BB_SUID_MAYBE, _BB_SUID_ALWAYS -}; +} bb_suid_t; +#endif -struct BB_applet { +struct bb_applet { const char *name; int (*main) (int argc, char **argv); - __extension__ enum Location location:8; - __extension__ enum SUIDRoot need_suid:8; +#if ENABLE_FEATURE_INSTALLER + __extension__ enum bb_install_loc_t install_loc:8; +#endif +#if ENABLE_FEATURE_SUID + __extension__ enum bb_suid_t need_suid:8; +#endif +#if ENABLE_FEATURE_EXEC_PREFER_APPLETS /* true if instead if fork(); exec("applet"); waitpid(); * one can do fork(); exit(applet_main(argc,argv)); waitpid(); */ unsigned char noexec; @@ -36,10 +45,11 @@ struct BB_applet { /* true if instead if fork(); exec("applet"); waitpid(); * one can simply call applet_main(argc,argv); */ unsigned char nofork; +#endif }; /* Defined in applet.c */ -extern const struct BB_applet applets[]; +extern const struct bb_applet applets[]; extern const unsigned short NUM_APPLETS; #endif /* _BB_INTERNAL_H_ */ diff --git a/include/libbb.h b/include/libbb.h index 3919524bc..6fff441c5 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -608,7 +608,7 @@ llist_t *llist_rev(llist_t *list); int write_pidfile(const char *path); #define remove_pidfile(f) ((void)unlink(f)) #else -#define write_pidfile(f) 1 +#define write_pidfile(f) TRUE #define remove_pidfile(f) ((void)0) #endif @@ -623,7 +623,7 @@ extern smallint logmode; extern int die_sleep; extern int xfunc_error_retval; extern jmp_buf die_jmp; -extern void sleep_and_die(void) ATTRIBUTE_NORETURN; +extern void xfunc_die(void) ATTRIBUTE_NORETURN; extern void bb_show_usage(void) ATTRIBUTE_NORETURN ATTRIBUTE_EXTERNALLY_VISIBLE; extern void bb_error_msg(const char *s, ...) __attribute__ ((format (printf, 1, 2))); extern void bb_error_msg_and_die(const char *s, ...) __attribute__ ((noreturn, format (printf, 1, 2))); @@ -670,8 +670,8 @@ const struct hwtype *get_hwntype(int type); #ifndef BUILD_INDIVIDUAL -struct BB_applet; -extern const struct BB_applet *find_applet_by_name(const char *name); +struct bb_applet; +extern const struct bb_applet *find_applet_by_name(const char *name); /* Returns only if applet is not found. */ extern void run_applet_by_name(const char *name, int argc, char **argv); extern void run_current_applet_and_exit(int argc, char **argv) ATTRIBUTE_NORETURN; @@ -880,7 +880,7 @@ enum { /* DO NOT CHANGE THESE VALUES! cp.c, mv.c, install.c depend on them. */ }; #define FILEUTILS_CP_OPTSTR "pdRfils" USE_SELINUX("c") -extern const struct BB_applet *current_applet; +extern const struct bb_applet *current_applet; extern const char *applet_name; extern const char BB_BANNER[]; diff --git a/libbb/copyfd.c b/libbb/copyfd.c index e0596d5f6..aa8fbb967 100644 --- a/libbb/copyfd.c +++ b/libbb/copyfd.c @@ -74,7 +74,7 @@ void complain_copyfd_and_die(off_t sz) if (sz != -1) bb_error_msg_and_die("short read"); /* if sz == -1, bb_copyfd_XX already complained */ - sleep_and_die(); + xfunc_die(); } #endif @@ -94,7 +94,7 @@ void bb_copyfd_exact_size(int fd1, int fd2, off_t size) if (sz != -1) bb_error_msg_and_die("short read"); /* if sz == -1, bb_copyfd_XX already complained */ - sleep_and_die(); + xfunc_die(); } off_t bb_copyfd_eof(int fd1, int fd2) diff --git a/libbb/error_msg_and_die.c b/libbb/error_msg_and_die.c index 39178a3ce..4a9049364 100644 --- a/libbb/error_msg_and_die.c +++ b/libbb/error_msg_and_die.c @@ -10,14 +10,25 @@ #include "libbb.h" int die_sleep; +#if ENABLE_FEATURE_EXEC_PREFER_APPLETS jmp_buf die_jmp; +#endif -void sleep_and_die(void) +void xfunc_die(void) { if (die_sleep) { - /* Special case: don't die, but jump */ - if (die_sleep < 0) - longjmp(die_jmp, xfunc_error_retval); + if (ENABLE_FEATURE_EXEC_PREFER_APPLETS && die_sleep < 0) { + /* Special case. We arrive here if NOFORK applet + * calls xfunc, which then decides to die. + * We don't die, but jump instead back to caller. + * NOFORK applets still cannot carelessly call xfuncs: + * p = xmalloc(10); + * q = xmalloc(10); // BUG! if this dies, we leak p! + */ + /* -111 means "zero" (longjmp can't pass 0) + * spawn_and_wait() catches -111. */ + longjmp(die_jmp, xfunc_error_retval ? xfunc_error_retval : -111); + } sleep(die_sleep); } exit(xfunc_error_retval); @@ -30,5 +41,5 @@ void bb_error_msg_and_die(const char *s, ...) va_start(p, s); bb_verror_msg(s, p, NULL); va_end(p); - sleep_and_die(); + xfunc_die(); } diff --git a/libbb/fflush_stdout_and_exit.c b/libbb/fflush_stdout_and_exit.c index ae68222b4..d79827f45 100644 --- a/libbb/fflush_stdout_and_exit.c +++ b/libbb/fflush_stdout_and_exit.c @@ -13,13 +13,17 @@ #include "libbb.h" -// TODO: make it safe to call from NOFORK applets -// Currently, it can exit(0). Even if it is made to do longjmp trick -// (see sleep_and_die internals), zero cannot be passed thru this way! - void fflush_stdout_and_exit(int retval) { if (fflush(stdout)) - sleep_and_die(); + xfunc_die(); + + if (ENABLE_FEATURE_EXEC_PREFER_APPLETS && die_sleep < 0) { + /* We are in NOFORK applet. Do not exit() directly, + * but use xfunc_die() */ + xfunc_error_retval = retval; + xfunc_die(); + } + exit(retval); } diff --git a/libbb/herror_msg_and_die.c b/libbb/herror_msg_and_die.c index a7a22caf7..8c77378d7 100644 --- a/libbb/herror_msg_and_die.c +++ b/libbb/herror_msg_and_die.c @@ -7,9 +7,6 @@ * Licensed under GPLv2 or later, see file LICENSE in this tarball for details. */ -#include -#include - #include "libbb.h" void bb_herror_msg_and_die(const char *s, ...) @@ -19,5 +16,5 @@ void bb_herror_msg_and_die(const char *s, ...) va_start(p, s); bb_vherror_msg(s, p); va_end(p); - sleep_and_die(); + xfunc_die(); } diff --git a/libbb/perror_msg_and_die.c b/libbb/perror_msg_and_die.c index 7521e7157..3a06b654b 100644 --- a/libbb/perror_msg_and_die.c +++ b/libbb/perror_msg_and_die.c @@ -7,10 +7,6 @@ * Licensed under GPLv2 or later, see file LICENSE in this tarball for details. */ -#include -#include -#include -#include #include "libbb.h" void bb_perror_msg_and_die(const char *s, ...) @@ -20,5 +16,5 @@ void bb_perror_msg_and_die(const char *s, ...) va_start(p, s); bb_vperror_msg(s, p); va_end(p); - sleep_and_die(); + xfunc_die(); } diff --git a/libbb/vfork_daemon_rexec.c b/libbb/vfork_daemon_rexec.c index 286ee2678..dabd1a6d6 100644 --- a/libbb/vfork_daemon_rexec.c +++ b/libbb/vfork_daemon_rexec.c @@ -16,7 +16,7 @@ */ #include -#include "busybox.h" /* for struct BB_applet */ +#include "busybox.h" /* for struct bb_applet */ /* This does a fork/exec in one call, using vfork(). Returns PID of new child, * -1 for failure. Runs argv[0], searching path if that has no / in it. */ @@ -104,8 +104,9 @@ int spawn_and_wait(char **argv) { int rc; - if (ENABLE_FEATURE_EXEC_PREFER_APPLETS) { - const struct BB_applet *a = find_applet_by_name(argv[0]); +#if ENABLE_FEATURE_EXEC_PREFER_APPLETS + { + const struct bb_applet *a = find_applet_by_name(argv[0]); if (a && (a->nofork #ifndef BB_NOMMU || a->noexec /* NOEXEC cannot be used on NOMMU */ @@ -120,19 +121,27 @@ int spawn_and_wait(char **argv) #endif { int old_sleep = die_sleep; + int old_x = xfunc_error_retval; die_sleep = -1; /* special flag */ - /* sleep_and_die() checks for it */ + /* xfunc_die() checks for it */ + rc = setjmp(die_jmp); if (!rc) { - const struct BB_applet *old_a = current_applet; + const struct bb_applet *old_a = current_applet; current_applet = a; applet_name = a->name; // what else should we save/restore? rc = a->main(argc, argv); current_applet = old_a; applet_name = old_a->name; + } else { + /* xfunc died in NOFORK applet */ + if (rc == -111) + rc = 0; } + die_sleep = old_sleep; + xfunc_error_retval = old_x; return rc; } #ifndef BB_NOMMU /* MMU only */ @@ -145,9 +154,13 @@ int spawn_and_wait(char **argv) run_current_applet_and_exit(argc, argv); #endif } + } rc = spawn(argv); w: +#else /* !FEATURE_EXEC_PREFER_APPLETS */ + rc = spawn(argv); +#endif /* FEATURE_EXEC_PREFER_APPLETS */ return wait4pid(rc); } diff --git a/libbb/xconnect.c b/libbb/xconnect.c index 8466325c7..e7d510678 100644 --- a/libbb/xconnect.c +++ b/libbb/xconnect.c @@ -166,7 +166,7 @@ USE_FEATURE_IPV6(sa_family_t af,) if (rc || !result) { bb_error_msg("bad address '%s'", org_host); if (ai_flags & DIE_ON_ERROR) - sleep_and_die(); + xfunc_die(); goto ret; } r = xmalloc(offsetof(len_and_sockaddr, sa) + result->ai_addrlen); diff --git a/libbb/xfuncs.c b/libbb/xfuncs.c index fa4a15236..b9d013a24 100644 --- a/libbb/xfuncs.c +++ b/libbb/xfuncs.c @@ -476,7 +476,7 @@ void xprint_and_close_file(FILE *file) fflush(stdout); // copyfd outputs error messages for us. if (bb_copyfd_eof(fileno(file), 1) == -1) - sleep_and_die(); + xfunc_die(); fclose(file); } diff --git a/shell/ash.c b/shell/ash.c index f98fc4178..b4278424a 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -6533,7 +6533,7 @@ tryexec(char *cmd, char **argv, char **envp) #if ENABLE_FEATURE_SH_STANDALONE_SHELL if (strchr(cmd, '/') == NULL) { - const struct BB_applet *a; + const struct bb_applet *a; a = find_applet_by_name(cmd); if (a) { diff --git a/shell/msh.c b/shell/msh.c index 3a5c85050..23a7c0498 100644 --- a/shell/msh.c +++ b/shell/msh.c @@ -3197,7 +3197,7 @@ static int dohelp(struct op *t) } #if ENABLE_FEATURE_SH_STANDALONE_SHELL { - const struct BB_applet *applet = applets; + const struct bb_applet *applet = applets; while (applet->name) { col += printf("%c%s", ((col == 0) ? '\t' : ' '), applet->name); diff --git a/util-linux/getopt.c b/util-linux/getopt.c index fefa02206..85a1d4410 100644 --- a/util-linux/getopt.c +++ b/util-linux/getopt.c @@ -51,10 +51,8 @@ enum { OPT_s = 0x10, // -s OPT_T = 0x20, // -T OPT_u = 0x40, // -u -#if ENABLE_GETOPT_LONG OPT_a = 0x80, // -a OPT_l = 0x100, // -l -#endif SHELL_IS_TCSH = 0x8000, /* hijack this bit for other purposes */ };