[PATCH v2] umount: Implement -O option to unmount by mount options
From: Sören Tempel This commit adds an implementation of the umount -O option, as provided by util-linux's mount(8) implementation, to BusyBox. Similar to -t, the option is intended to be used in conjunction with -a thereby allowing users to filter which file systems are unmounted by mount options. Multiple options can be specified with -O, all of which need to match. Each option can be prefixed with `no` to indicate that no action should be taken for a mount point with this mount option. The "no" prefix interpretation can be disabled using the "+" prefix. At Alpine, this feature is often requested by users as the OpenRC netmount service uses `umount -a -O _netdev` to amount all network file systems [1] [2]. This implementation is functionally equivalent to the util-linux implementation with the exception that it implements no special handling for `key="value"` mount options to keep the implementation simple. Therefore, filesystems mounted with options like `foo="bar"` won't be matched by `umount -a -O foo`. [1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/9923 [2]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13789 Signed-off-by: Sören Tempel --- Changes since v1: * Fix a typo during parsing of no keyword (n0 -> no) * Treat alone "no" as an error (as done in util-linux) * Implement support for the "+" prefix to bypass the "no" keyword This version of the patch has been tested against the util-linux implementation using KLEE: https://github.com/nmeum/umount-test-opts include/libbb.h | 1 + libbb/Kbuild.src | 1 + libbb/match_fsopts.c | 69 util-linux/umount.c | 10 +-- 4 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 libbb/match_fsopts.c diff --git a/include/libbb.h b/include/libbb.h index cca33a177..ad41adec8 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -1586,6 +1586,7 @@ const struct hwtype *get_hwntype(int type) FAST_FUNC; extern int fstype_matches(const char *fstype, const char *comma_list) FAST_FUNC; +extern int fsopts_matches(const char *opts_list, const char *reqopts_list) FAST_FUNC; #ifdef HAVE_MNTENT_H extern struct mntent *find_mount_point(const char *name, int subdir_too) FAST_FUNC; #endif diff --git a/libbb/Kbuild.src b/libbb/Kbuild.src index 653025e56..4bb8260b9 100644 --- a/libbb/Kbuild.src +++ b/libbb/Kbuild.src @@ -120,6 +120,7 @@ lib-y += xrealloc_vector.o lib-$(CONFIG_MOUNT) += match_fstype.o lib-$(CONFIG_UMOUNT) += match_fstype.o +lib-$(CONFIG_UMOUNT) += match_fsopts.o lib-$(CONFIG_FEATURE_UTMP) += utmp.o diff --git a/libbb/match_fsopts.c b/libbb/match_fsopts.c new file mode 100644 index 0..b1cc85c3c --- /dev/null +++ b/libbb/match_fsopts.c @@ -0,0 +1,69 @@ +/* vi: set sw=4 ts=4: */ +/* + * Match fsopts for use in mount unmount -O. + * + * Returns 1 for a match, otherwise 0. + * + * Licensed under GPLv2 or later, see file LICENSE in this source tree. + */ + +#include "libbb.h" + +static int fsopt_matches(const char *opts_list, const char *opt, size_t optlen) +{ + int match = 1; + + if (optlen >= 2 && opt[0] == 'n' && opt[1] == 'o') { + match--; + opt += 2; optlen -= 2; + } + + /* The alone "no" is an error, all matching ends with False. */ + if (optlen == 0) + return 0; + + /* The "no" prefix interpretation can be disabled by the "+" prefix. */ + if (match && optlen > 1 && *opt == '+') { + opt++; optlen--; + } + + while (1) { + if (strncmp(opts_list, opt, optlen) == 0) { + const char *after_opt = opts_list + optlen; + if (*after_opt == '\0' || *after_opt == ',') + return match; + } + + opts_list = strchr(opts_list, ','); + if (!opts_list) + break; + opts_list++; + } + + return !match; +} + +/* This function implements the mnt_match_options function from libmount. */ +int FAST_FUNC fsopts_matches(const char *opts_list, const char *reqopts_list) +{ + if (!reqopts_list) + return 1; /* no options requested, match anything */ + + while (1) { + size_t len; + const char *comma = strchr(reqopts_list, ','); + if (!comma) + len = strlen(reqopts_list); + else + len = comma - reqopts_list; + + if (len && !fsopt_matches(opts_list, reqopts_list, len)) + return 0; + + if (!comma) + break; + reqopts_list = ++comma; + } + + return 1; +} diff --git a/util-linux/umount.c b/util-linux/umount.c index 23da32868..7a54cafb0 100644 --- a/util-linux/umount.c +++ b/util-linux/umount.c @@ -41,7 +41,7 @@ //kbuild:lib-$(CONFIG_UMOUNT) += umount.o
[PATCH] umount: Implement -O option to unmount by mount options
From: Sören Tempel This commit adds a primitive implementation of the umount -O option, as provided by util-linux's mount(8) implementation, to BusyBox. Similar to -t, the option is intended to be used in conjunction with -a thereby allowing users to filter which file systems are unmounted by mount options. Multiple options can be specified with -O, all of which need to match. Each option can be prefixed with `no` to indicate that no action should be taken for a mount point with this mount option. At Alpine, this feature is often requested by users as the OpenRC netmount service uses `umount -a -O _netdev` to amount all network file systems [1] [2]. Discussion: * There is some minor code duplication between fsopt_matches and fstype_matches. Adding some sort of utility function to resolve this may allow for a further decrease in text segment size. * The semantics of -O are not well described in the util-linux mount(8) man page. Please review this carefully to ensure that the implementation proposed here is semantically equivalent to the one provided by util-linux. [1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/9923 [2]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13789 Signed-off-by: Sören Tempel --- I haven't tested this extensively yet. Feedback is most welcome. include/libbb.h | 1 + libbb/Kbuild.src | 1 + libbb/match_fsopts.c | 59 util-linux/umount.c | 10 +--- 4 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 libbb/match_fsopts.c diff --git a/include/libbb.h b/include/libbb.h index 6aeec249d..1a203861e 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -1585,6 +1585,7 @@ const struct hwtype *get_hwntype(int type) FAST_FUNC; extern int fstype_matches(const char *fstype, const char *comma_list) FAST_FUNC; +extern int fsopts_matches(const char *opts_list, const char *reqopts_list) FAST_FUNC; #ifdef HAVE_MNTENT_H extern struct mntent *find_mount_point(const char *name, int subdir_too) FAST_FUNC; #endif diff --git a/libbb/Kbuild.src b/libbb/Kbuild.src index 653025e56..4bb8260b9 100644 --- a/libbb/Kbuild.src +++ b/libbb/Kbuild.src @@ -120,6 +120,7 @@ lib-y += xrealloc_vector.o lib-$(CONFIG_MOUNT) += match_fstype.o lib-$(CONFIG_UMOUNT) += match_fstype.o +lib-$(CONFIG_UMOUNT) += match_fsopts.o lib-$(CONFIG_FEATURE_UTMP) += utmp.o diff --git a/libbb/match_fsopts.c b/libbb/match_fsopts.c new file mode 100644 index 0..fff236c7a --- /dev/null +++ b/libbb/match_fsopts.c @@ -0,0 +1,59 @@ +/* vi: set sw=4 ts=4: */ +/* + * Match fsopts for use in mount unmount -O. + * + * Returns 1 for a match, otherwise 0. + * + * Licensed under GPLv2 or later, see file LICENSE in this source tree. + */ + +#include "libbb.h" + +static int FAST_FUNC fsopt_matches(const char *opts_list, const char *opt, size_t optlen) +{ + int match = 1; + + if (optlen > 2 && opt[0] == 'n' && opt[1] == '0') { + match--; + opt += 2; optlen -= 2; + } + + while (1) { + if (strncmp(opts_list, opt, optlen) == 0) { + const char *after_opt = opts_list + optlen; + if (*after_opt == '\0' || *after_opt == ',') + return match; + } + + opts_list = strchr(opts_list, ','); + if (!opts_list) + break; + opts_list++; + } + + return !match; +} + +int FAST_FUNC fsopts_matches(const char *opts_list, const char *reqopts_list) +{ + if (!reqopts_list) + return 1; /* no options requested, match anything */ + + while (1) { + size_t len; + const char *comma = strchr(reqopts_list, ','); + if (!comma) + len = strlen(reqopts_list); + else + len = comma - reqopts_list; + + if (len && !fsopt_matches(opts_list, reqopts_list, len)) + return 0; + + if (!comma) + break; + reqopts_list = ++comma; + } + + return 1; +} diff --git a/util-linux/umount.c b/util-linux/umount.c index 23da32868..7a54cafb0 100644 --- a/util-linux/umount.c +++ b/util-linux/umount.c @@ -41,7 +41,7 @@ //kbuild:lib-$(CONFIG_UMOUNT) += umount.o //usage:#define umount_trivial_usage -//usage: "[-rlf"IF_FEATURE_MTAB_SUPPORT("m")IF_FEATURE_MOUNT_LOOP("d")IF_FEATURE_UMOUNT_ALL("a")"] [-t FSTYPE] FILESYSTEM|DIRECTORY" +//usage: "[-rlf"IF_FEATURE_MTAB_SUPPORT("m")IF_FEATURE_MOUNT_LOOP("d")IF_FEATURE_UMOUNT_ALL("a")"] [-t FSTYPE] [-O FSOPT] FILESYSTEM|DIRECTORY" //usage:#define umount_full_usage "\n\n" //usage: "Unmount filesystems\n" //usage: IF_FEATURE_UMOUNT_ALL( @@ -57,6 +57,7 @@ //usage: "\n -d Free loop device if it has been used" //usage: ) //usage: "\n
[PATCH v2] ash: Fix use-after-free on idx variable
From: Sören Tempel Consider the following code from ash.c: STPUTC(*idx, expdest); if (quotes && (unsigned char)*idx == CTLESC) { The idx variable points to a value in the stack string (as managed by STPUTC). STPUTC may resize this stack string via realloc(3). If this happens, the idx pointer needs to be updated. Otherwise, dereferencing idx may result in a use-after free. The valgrind output for this edge case looks as follows: Invalid read of size 1 at 0x113AD7: subevalvar (ash.c:7326) by 0x112EC7: evalvar (ash.c:7674) by 0x113219: argstr (ash.c:6891) by 0x113D10: expandarg (ash.c:8098) by 0x118989: evalcommand (ash.c:10377) by 0x116744: evaltree (ash.c:9373) by 0x1170DC: cmdloop (ash.c:13577) by 0x1191E4: ash_main (ash.c:14756) by 0x10CB3B: run_applet_no_and_exit (appletlib.c:967) by 0x10CBCA: run_applet_and_exit (appletlib.c:986) by 0x10CBCA: main (appletlib.c:1126) Address 0x48b4099 is 857 bytes inside a block of size 2,736 free'd at 0x48A6FC9: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x125B03: xrealloc (xfuncs_printf.c:61) by 0x10F9D2: growstackblock (ash.c:1736) by 0x10FA4E: growstackstr (ash.c:1775) by 0x10FA71: _STPUTC (ash.c:1816) by 0x113A94: subevalvar (ash.c:7325) by 0x112EC7: evalvar (ash.c:7674) by 0x113219: argstr (ash.c:6891) by 0x113D10: expandarg (ash.c:8098) by 0x118989: evalcommand (ash.c:10377) by 0x116744: evaltree (ash.c:9373) by 0x1170DC: cmdloop (ash.c:13577) Block was alloc'd at at 0x48A26D5: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x125AE9: xmalloc (xfuncs_printf.c:50) by 0x10ED56: stalloc (ash.c:1622) by 0x10F9FF: growstackblock (ash.c:1746) by 0x10FB2A: growstackto (ash.c:1783) by 0x10FB47: makestrspace (ash.c:1795) by 0x10FDE7: memtodest (ash.c:6390) by 0x10FE91: strtodest (ash.c:6417) by 0x112CC5: varvalue (ash.c:7558) by 0x112D80: evalvar (ash.c:7603) by 0x113219: argstr (ash.c:6891) by 0x113D10: expandarg (ash.c:8098) This patch fixes this issue by updating the pointers again via the restart label if STPUTC re-sized the stack. This issue has been reported to us at Alpine Linux downstream. Also: Move the second realloc-check inside the if statement that follows so it isn't done twice if the condition evaluates to false. See also: * https://gitlab.alpinelinux.org/alpine/aports/-/issues/13900 * http://lists.busybox.net/pipermail/busybox/2022-April/089655.html --- Changes since v1: Don't check for restart twice in a row if `quotes && (unsigned char)*idx == CTLESC` evaluates to false. shell/ash.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index ef4a47afe..cbc50eefe 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -7323,13 +7323,15 @@ subevalvar(char *start, char *str, int strloc, if (idx >= end) break; STPUTC(*idx, expdest); + if (stackblock() != restart_detect) + goto restart; if (quotes && (unsigned char)*idx == CTLESC) { idx++; len++; STPUTC(*idx, expdest); + if (stackblock() != restart_detect) + goto restart; } - if (stackblock() != restart_detect) - goto restart; idx++; len++; rmesc++; ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ash: Fix use-after-free on idx variable
From: Sören Tempel Consider the following code from ash.c: STPUTC(*idx, expdest); if (quotes && (unsigned char)*idx == CTLESC) { The idx variable points to a value in the stack string (as managed by STPUTC). STPUTC may resize this stack string via realloc(3). If this happens, the idx pointer needs to be updated. Otherwise, dereferencing idx may result in a use-after free. The valgrind output for this edge case looks as follows: Invalid read of size 1 at 0x113AD7: subevalvar (ash.c:7326) by 0x112EC7: evalvar (ash.c:7674) by 0x113219: argstr (ash.c:6891) by 0x113D10: expandarg (ash.c:8098) by 0x118989: evalcommand (ash.c:10377) by 0x116744: evaltree (ash.c:9373) by 0x1170DC: cmdloop (ash.c:13577) by 0x1191E4: ash_main (ash.c:14756) by 0x10CB3B: run_applet_no_and_exit (appletlib.c:967) by 0x10CBCA: run_applet_and_exit (appletlib.c:986) by 0x10CBCA: main (appletlib.c:1126) Address 0x48b4099 is 857 bytes inside a block of size 2,736 free'd at 0x48A6FC9: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x125B03: xrealloc (xfuncs_printf.c:61) by 0x10F9D2: growstackblock (ash.c:1736) by 0x10FA4E: growstackstr (ash.c:1775) by 0x10FA71: _STPUTC (ash.c:1816) by 0x113A94: subevalvar (ash.c:7325) by 0x112EC7: evalvar (ash.c:7674) by 0x113219: argstr (ash.c:6891) by 0x113D10: expandarg (ash.c:8098) by 0x118989: evalcommand (ash.c:10377) by 0x116744: evaltree (ash.c:9373) by 0x1170DC: cmdloop (ash.c:13577) Block was alloc'd at at 0x48A26D5: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x125AE9: xmalloc (xfuncs_printf.c:50) by 0x10ED56: stalloc (ash.c:1622) by 0x10F9FF: growstackblock (ash.c:1746) by 0x10FB2A: growstackto (ash.c:1783) by 0x10FB47: makestrspace (ash.c:1795) by 0x10FDE7: memtodest (ash.c:6390) by 0x10FE91: strtodest (ash.c:6417) by 0x112CC5: varvalue (ash.c:7558) by 0x112D80: evalvar (ash.c:7603) by 0x113219: argstr (ash.c:6891) by 0x113D10: expandarg (ash.c:8098) This patch fixes this issue by updating the pointers again via the restart label if STPUTC re-sized the stack. This issue has been reported to us at Alpine Linux downstream. See also: * https://gitlab.alpinelinux.org/alpine/aports/-/issues/13900 * http://lists.busybox.net/pipermail/busybox/2022-April/089655.html --- shell/ash.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shell/ash.c b/shell/ash.c index ef4a47afe..924729a64 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -7323,6 +7323,8 @@ subevalvar(char *start, char *str, int strloc, if (idx >= end) break; STPUTC(*idx, expdest); + if (stackblock() != restart_detect) + goto restart; if (quotes && (unsigned char)*idx == CTLESC) { idx++; len++; ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ash: don't read past end of var in subvareval for bash substitutions
From: Sören Tempel Without this patch, BusyBox handles bash pattern substitutions without a terminating '/' character incorrectly. Consider the following shell script: _bootstrapver=5.0.211-r0 _referencesdir="/usr/${_bootstrapver/-*}/Sources" echo $_referencesdir This should output `/usr/5.0.211/Sources`. However, without this patch it instead outputs `/usr/5.0.211Sources`. This is due to the fact that BusyBox expects the bash pattern substitutions to always be terminated with a '/' (at least in this part of subvareval) and thus reads passed the substitution itself and consumes the '/' character which is part of the literal string. If there is no '/' after the substitution then BusyBox might perform an out-of-bounds read under certain circumstances. When replacing the bash pattern substitution with `${_bootstrapver/-*/}`, or with this patch applied, ash outputs the correct value. Signed-off-by: Sören Tempel --- shell/ash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/ash.c b/shell/ash.c index adb0f223a..8097d51c3 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -7077,7 +7077,7 @@ subevalvar(char *start, char *str, int strloc, repl = NULL; break; } - if (*repl == '/') { + if (*repl == '/' || (unsigned char)*repl == CTLENDVAR) { *repl = '\0'; break; } ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ed: don't use memcpy with overlapping memory regions
From: Sören Tempel The memcpy invocations in the subCommand function, modified by this commit, previously used memcpy with overlapping memory regions. This is undefined behavior. On Alpine Linux, it causes BusyBox ed to crash since we compile BusyBox with -D_FORTIFY_SOURCE=2 and our fortify-headers implementation catches this source of undefined behavior [0]. The issue can only be triggered if the replacement string is the same size or shorter than the old string. Looking at the code, it seems to me that a memmove(3) is what was actually intended here, this commit modifies the code accordingly. [0]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13504 --- editors/ed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editors/ed.c b/editors/ed.c index 209ce9942..4a84f7433 100644 --- a/editors/ed.c +++ b/editors/ed.c @@ -720,7 +720,7 @@ static void subCommand(const char *cmd, int num1, int num2) if (deltaLen <= 0) { memcpy(>data[offset], newStr, newLen); if (deltaLen) { - memcpy(>data[offset + newLen], + memmove(>data[offset + newLen], >data[offset + oldLen], lp->len - offset - oldLen); ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ash: fix use-after-free in bash pattern substitution
From: Sören Tempel At Alpine Linux downstream, we were made aware of a segmentation fault occurring during string replacement in BusyBox ash [0]. Further debugging revealed that the segmentation fault occurs due to a use-after-free in BusyBox's bash pattern substitution implementation. Specially, the problem is that the repl variable (pointing to the replacement string) points to a value in the stack string. However, when accessing the repl pointer in Line 7350 it is possible that the stack has been moved since the last repl assignment due to the STPUTC invocations in Line 7317 and 7321 (since STPUTC may grow the stack via realloc(3)). For this reason, the code in Line 7350 may access an unmapped memory region and therefore causes a segmentation fault if prior STPUTC invocations moved the stack via realloc(3). The valgrind output for this edge case looks as follows: Invalid read of size 1 at 0x15D8DD: subevalvar (ash.c:7350) by 0x15DC43: evalvar (ash.c:7666) by 0x15B717: argstr (ash.c:6893) by 0x15BAEC: expandarg (ash.c:8090) by 0x15F4CC: evalcommand (ash.c:10429) by 0x15B26C: evaltree (ash.c:9365) by 0x15E4FC: cmdloop (ash.c:13569) by 0x15FD8B: ash_main (ash.c:14748) by 0x115BF2: run_applet_no_and_exit (appletlib.c:967) by 0x115F16: run_applet_and_exit (appletlib.c:986) by 0x115EF9: busybox_main (appletlib.c:917) by 0x115EF9: run_applet_and_exit (appletlib.c:979) by 0x115F8F: main (appletlib.c:1126) Address 0x48b8646 is 2,054 bytes inside a block of size 4,776 free'd at 0x48A6FC9: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x116E86: xrealloc (xfuncs_printf.c:61) by 0x1565DB: growstackblock (ash.c:1736) by 0x156EF7: growstackstr (ash.c:1775) by 0x156F1A: _STPUTC (ash.c:1816) by 0x15D843: subevalvar (ash.c:7317) by 0x15DC43: evalvar (ash.c:7666) by 0x15B717: argstr (ash.c:6893) by 0x15BAEC: expandarg (ash.c:8090) by 0x15F4CC: evalcommand (ash.c:10429) by 0x15B26C: evaltree (ash.c:9365) by 0x15E4FC: cmdloop (ash.c:13569) A testcase for reproducing this edge case is provided in the downstream bug report [1]. This commit fixes the issue by reconstructing the repl pointer relative to stackblock() via strloc and slash_pos. [0]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13469 [1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13469#note_210530 Signed-off-by: Sören Tempel --- Discussion: I am not familiar with the ash code base. For this reason, it is presently unclear to me if there is a path where slash_pos < 0, STPUTC is invoked, and repl points to the stack. If so, handling for the case that slash_pos < 0 also needs to be added to the proposed path. Furthermore, I haven't tested this patch extensively so please review with extra care. shell/ash.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/shell/ash.c b/shell/ash.c index 55df54bd0..24f9a8270 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -7346,6 +7346,12 @@ subevalvar(char *start, char *str, int strloc, idx = loc; } + // The STPUTC invocations above may resize and move the + // stack via realloc(3). Since repl is a pointer into the + // stack, we need to reconstruct it relative to stackblock(). + if (slash_pos >= 0) + repl = (char *)stackblock() + strloc + slash_pos + 1; + //bb_error_msg("repl:'%s'", repl); for (loc = (char*)repl; *loc; loc++) { char *restart_detect = stackblock(); ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2] ed: add support for -s command-line option as mandated by POSIX
From: Sören Tempel Apart from the -p option, POSIX also mandates an -s option which suppresses the output of byte counts for the e, E, r, and w command. From these commands, Busybox ed presently only implements the r and w commands. This commit ensures that these two command do not output any bytes counts when the -s option is passed. The shell escape command, also effected by the -s option, is not implemented by Busybox at the moment. --- Instead of introducing a new global variable, this version of the patch reuses the existing option_mask32 variable. editors/ed.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/editors/ed.c b/editors/ed.c index dfe0f1a77..36b045733 100644 --- a/editors/ed.c +++ b/editors/ed.c @@ -18,7 +18,7 @@ //applet:IF_ED(APPLET(ed, BB_DIR_BIN, BB_SUID_DROP)) -//usage:#define ed_trivial_usage "[-p PROMPT] [FILE]" +//usage:#define ed_trivial_usage "[-p PROMPT] [-s] [FILE]" //usage:#define ed_full_usage "" #include "libbb.h" @@ -71,6 +71,11 @@ struct globals { SET_PTR_TO_GLOBALS(xzalloc(sizeof(G))); \ } while (0) +#define OPTION_STR "sp:" +enum { + OPT_s = 0x01, +}; + static int bad_nums(int num1, int num2, const char *for_what) { if ((num1 < 1) || (num2 > lastNum) || (num1 > num2)) { @@ -458,7 +463,8 @@ static int readLines(const char *file, int num) * in the following format: * "%d\n", */ - printf("%u\n", charCount); + if (!(option_mask32 & OPT_s)) + printf("%u\n", charCount); return TRUE; } @@ -510,7 +516,8 @@ static int writeLines(const char *file, int num1, int num2) * unless the -s option was specified, in the following format: * "%d\n", */ - printf("%u\n", charCount); + if (!(option_mask32 & OPT_s)) + printf("%u\n", charCount); return TRUE; } @@ -1005,7 +1012,7 @@ int ed_main(int argc UNUSED_PARAM, char **argv) lines.prev = prompt = ""; /* no prompt by default */ - getopt32(argv, "p:", ); + getopt32(argv, OPTION_STR, ); argv += optind; if (argv[0]) { ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2] ed: add support for -p command-line option as mandated by POSIX
From: Sören Tempel The POSIX.1-2008 specification of ed(1) mandates two command-line options: -p (for specifying a prompt string) and -s (to suppress writing of byte counts). This commit adds support for the former. Furthermore, it also changes the default prompt string to an empty string (instead of ": ") since this is also mandated by POSIX: -p string Use string as the prompt string when in command mode. By default, there shall be no prompt string. Support for the remaining -s option will be added in a separate commit since it requires a general restructuring of error handling in Busybox ed. Signed-off-by: Sören Tempel --- editors/ed.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/editors/ed.c b/editors/ed.c index 0d96d263c..a54a0b0a6 100644 --- a/editors/ed.c +++ b/editors/ed.c @@ -48,6 +48,7 @@ struct globals { char *bufBase; char *bufPtr; char *fileName; + const char *prompt; LINE lines; smallint dirty; int marks[26]; @@ -57,6 +58,7 @@ struct globals { #define bufBase(G.bufBase ) #define bufPtr (G.bufPtr) #define fileName (G.fileName ) +#define prompt (G.prompt) #define curNum (G.curNum) #define lastNum(G.lastNum ) #define bufUsed(G.bufUsed ) @@ -790,7 +792,7 @@ static void doCommands(void) * 0 on ctrl-C, * >0 length of input string, including terminating '\n' */ - len = read_line_input(NULL, ": ", buf, sizeof(buf)); + len = read_line_input(NULL, prompt, buf, sizeof(buf)); if (len <= 0) return; while (len && isspace(buf[--len])) @@ -994,6 +996,8 @@ static void doCommands(void) int ed_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int ed_main(int argc UNUSED_PARAM, char **argv) { + int opt; + INIT_G(); bufSize = INITBUF_SIZE; @@ -1002,8 +1006,15 @@ int ed_main(int argc UNUSED_PARAM, char **argv) lines.next = lines.prev = - if (argv[1]) { - fileName = xstrdup(argv[1]); + opt = getopt32(argv, "p:", ); + if (!(opt & 0x01)) + prompt = ""; /* no prompt by default */ + + argc -= optind; + argv += optind; + + if (argc >= 1) { + fileName = xstrdup(argv[0]); if (!readLines(fileName, 1)) { return EXIT_SUCCESS; } ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ed: add support for -p command-line option as mandated by POSIX
From: Sören Tempel The POSIX.1-2008 specification of ed(1) mandates two command-line options: -p (for specifying a prompt string) and -s (to suppress writing of byte counts). This commit adds support for the former. Furthermore, it also changes the default prompt string to an empty string (instead of ": ") since this is also mandated by POSIX: -p string Use string as the prompt string when in command mode. By default, there shall be no prompt string. Support for the remaining -s option will be added in a separate commit since it requires a general restructuring of error handling in Busybox ed. Signed-off-by: Sören Tempel --- editors/ed.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/editors/ed.c b/editors/ed.c index 0d96d263c..6473281a1 100644 --- a/editors/ed.c +++ b/editors/ed.c @@ -48,6 +48,7 @@ struct globals { char *bufBase; char *bufPtr; char *fileName; + char *prompt; LINE lines; smallint dirty; int marks[26]; @@ -57,6 +58,7 @@ struct globals { #define bufBase(G.bufBase ) #define bufPtr (G.bufPtr) #define fileName (G.fileName ) +#define prompt (G.prompt) #define curNum (G.curNum) #define lastNum(G.lastNum ) #define bufUsed(G.bufUsed ) @@ -790,7 +792,7 @@ static void doCommands(void) * 0 on ctrl-C, * >0 length of input string, including terminating '\n' */ - len = read_line_input(NULL, ": ", buf, sizeof(buf)); + len = read_line_input(NULL, prompt, buf, sizeof(buf)); if (len <= 0) return; while (len && isspace(buf[--len])) @@ -994,6 +996,8 @@ static void doCommands(void) int ed_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int ed_main(int argc UNUSED_PARAM, char **argv) { + int opt; + INIT_G(); bufSize = INITBUF_SIZE; @@ -1002,8 +1006,15 @@ int ed_main(int argc UNUSED_PARAM, char **argv) lines.next = lines.prev = - if (argv[1]) { - fileName = xstrdup(argv[1]); + opt = getopt32(argv, "p:", ); + if (!(opt & 0x1)) + prompt = xstrdup(""); /* no prompt by default */ + + argc -= optind; + argv += optind; + + if (argc >= 1) { + fileName = xstrdup(argv[0]); if (!readLines(fileName, 1)) { return EXIT_SUCCESS; } ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ed: align output of read command with POSIX.1-2008
From: Sören Tempel POSIX.1-2008 mandates the following regarding the read command: If the read is successful, and -s was not specified, the number of bytes read shall be written to standard output in the following format: "%d\n", This commit aligns the output of busybox ed with POSIX.1-2008 by removing the file name from the output for the read command. This slipped through in 4836a0708fd0aaeb82871a3762b40fcf4b61e812. Signed-off-by: Sören Tempel --- editors/ed.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/editors/ed.c b/editors/ed.c index cb903bca7..0d96d263c 100644 --- a/editors/ed.c +++ b/editors/ed.c @@ -400,9 +400,6 @@ static int readLines(const char *file, int num) charCount = 0; cc = 0; - printf("\"%s\", ", file); - fflush_all(); - do { cp = memchr(bufPtr, '\n', bufUsed); ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ed: fix current line number for file passed via the command-line
From: Sören Tempel POSIX.1-2008 mandates the following regarding the file command-line argument: If the file argument is given, ed shall simulate an e command on the file named by the pathname […] The specification for the e command mandates the following behaviour regarding the current line number in POSIX.1-2008: The current line number shall be set to the address of the last line of the buffer. However, without this commit, busybox ed will set the current line number to 1 if a file is given on the command-line and this file is not empty (lastNum != 0). This is incorrect and fixed in this commit by not modifying the current line number in ed_main(). As such, the current line number will be zero for empty files and otherwise be set to the address of the last line of the buffer. Signed-off-by: Sören Tempel --- editors/ed.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/editors/ed.c b/editors/ed.c index 14540e566..cb903bca7 100644 --- a/editors/ed.c +++ b/editors/ed.c @@ -1010,8 +1010,6 @@ int ed_main(int argc UNUSED_PARAM, char **argv) if (!readLines(fileName, 1)) { return EXIT_SUCCESS; } - if (lastNum) - setCurNum(1); dirty = FALSE; } ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ed: align output of write command with POSIX-1.2008
From: Sören Tempel POSIX.1-2008 mandates the following regarding the write command: If the command is successful, the number of bytes written shall be written to standard output, unless the -s option was specified, in the following format: "%d\n", Presently, busybox does not use this specified format, the patch proposed here aligns the output format with POSIX.1-2008. I was unable to infer from git-blame(1) why this deviation from the standard was added in the first please, though I am aware that busybox ed is not POSIX compatible anyhow in its present form. For instance, it does also not support the aforementioned -s option. The difference in output behavior of the write command is just one deviation from the POSIX standard I just happend to came across. If you are not striving toward making busybox ed compatible with POSIX anyhow, please let me know. --- editors/ed.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/editors/ed.c b/editors/ed.c index c50faeefa..3b54f274b 100644 --- a/editors/ed.c +++ b/editors/ed.c @@ -468,12 +468,11 @@ static int readLines(const char *file, int num) static int writeLines(const char *file, int num1, int num2) { LINE *lp; - int fd, lineCount, charCount; + int fd, charCount; if (bad_nums(num1, num2, "write")) return FALSE; - lineCount = 0; charCount = 0; fd = creat(file, 0666); @@ -482,9 +481,6 @@ static int writeLines(const char *file, int num1, int num2) return FALSE; } - printf("\"%s\", ", file); - fflush_all(); - lp = findLine(num1); if (lp == NULL) { close(fd); @@ -498,7 +494,6 @@ static int writeLines(const char *file, int num1, int num2) return FALSE; } charCount += lp->len; - lineCount++; lp = lp->next; } @@ -507,7 +502,7 @@ static int writeLines(const char *file, int num1, int num2) return FALSE; } - printf("%d lines, %d chars\n", lineCount, charCount); + printf("%d\n", charCount); return TRUE; } ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ls: Don't output any colors with TERM=dumb
From: Sören Tempel The TERM variable is usually set to dumb to indicate that the terminal does not support any ANSI escape sequences. Presently, ls does not honor this variable and outputs colors anyhow which results in unreadable output, unless the user explicitly disables colors using `ls --color=never`. The rational behind this change is that ls should "just work" by default, even on dumb terminals. For this reason, this patch adds a check which additionally consults the TERM variable before printing any colors. This is analogous to the existing check for ensuring that standard output is a tty. As such, colors can still be forced with `--color=force`, even if TERM is set to dumb. The implementation includes a new libbb function is_dumb_term() since (a) this code may be useful in other applets (e.g. busybox ash) which also use ANSI escape sequences unconditionally and (b) it makes it easier to add additional heuristics for identifying dumb terminals in the future. --- coreutils/ls.c | 4 ++-- include/libbb.h | 1 + libbb/xfuncs.c | 6 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/coreutils/ls.c b/coreutils/ls.c index 80ef92079..a11a92456 100644 --- a/coreutils/ls.c +++ b/coreutils/ls.c @@ -1145,7 +1145,7 @@ int ls_main(int argc UNUSED_PARAM, char **argv) #if ENABLE_FEATURE_LS_COLOR /* set G_show_color = 1/0 */ - if (ENABLE_FEATURE_LS_COLOR_IS_DEFAULT && isatty(STDOUT_FILENO)) { + if (ENABLE_FEATURE_LS_COLOR_IS_DEFAULT && isatty(STDOUT_FILENO) && !is_dumb_term()) { char *p = getenv("LS_COLORS"); /* LS_COLORS is unset, or (not empty && not "none") ? */ if (!p || (p[0] && strcmp(p, "none") != 0)) @@ -1158,7 +1158,7 @@ int ls_main(int argc UNUSED_PARAM, char **argv) case 3: case 4: case 5: - if (isatty(STDOUT_FILENO)) { + if (isatty(STDOUT_FILENO) && !is_dumb_term()) { case 0: case 1: case 2: diff --git a/include/libbb.h b/include/libbb.h index 03f9c35f3..9085a05e9 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -1773,6 +1773,7 @@ extern void print_login_issue(const char *issue_file, const char *tty) FAST_FUNC extern void print_login_prompt(void) FAST_FUNC; char *xmalloc_ttyname(int fd) FAST_FUNC RETURNS_MALLOC; +int is_dumb_term(void); /* NB: typically you want to pass fd 0, not 1. Think 'applet | grep something' */ int get_terminal_width_height(int fd, unsigned *width, unsigned *height) FAST_FUNC; int get_terminal_width(int fd) FAST_FUNC; diff --git a/libbb/xfuncs.c b/libbb/xfuncs.c index d93d8aaf5..c81ce4546 100644 --- a/libbb/xfuncs.c +++ b/libbb/xfuncs.c @@ -303,6 +303,12 @@ int FAST_FUNC get_terminal_width(int fd) return width; } +int FAST_FUNC is_dumb_term(void) +{ + char *term = getenv("TERM"); + return term && strcmp(term, "dumb") == 0; +} + int FAST_FUNC tcsetattr_stdin_TCSANOW(const struct termios *tp) { return tcsetattr(STDIN_FILENO, TCSANOW, tp); ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2] deluser: check if specified home is a directory before removing it
From: Sören Tempel On Alpine, some users use /dev/null as a home directory. When removing such a user with `deluser --remove-home` this causes the /dev/null device file to be removed which is undesirable. To prevent this pitfall, check if the home directory specified for the user is an actual directory (or a symlink to a directory). Implementations of similar tools for other operating systems also implement such checks. For instance, the OpenBSD rmuser(1) implementation [0]. [0]: https://github.com/openbsd/src/blob/b69faa6c70c5bfcfdddc6138cd8e0ee18cc15b03/usr.sbin/adduser/rmuser.perl#L143-L151 --- loginutils/deluser.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/loginutils/deluser.c b/loginutils/deluser.c index 56bc7eaa6..585e82090 100644 --- a/loginutils/deluser.c +++ b/loginutils/deluser.c @@ -99,8 +99,15 @@ int deluser_main(int argc, char **argv) pfile = bb_path_passwd_file; if (ENABLE_FEATURE_SHADOWPASSWDS) sfile = bb_path_shadow_file; - if (opt_delhome) - remove_file(pw->pw_dir, FILEUTILS_RECUR); + if (opt_delhome) { + struct stat st; + + /* Make sure home is an actual directory before +* removing it (e.g. users with /dev/null as home) */ + xstat(pw->pw_dir, ); + if (S_ISDIR(st.st_mode)) + remove_file(pw->pw_dir, FILEUTILS_RECUR); + } } else { struct group *gr; do_delgroup: ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] deluser: check if specified home is a directory before removing it
From: Sören Tempel On Alpine, some users use /dev/null as a home directory. When removing such a user with `deluser --remove-home` this causes the /dev/null device file to be removed which is undesirable. To prevent this pitfall, check if the home directory specified for the user is an actual directory (or a symlink to a directory). Implementations of similar tools for other operating systems also implement such checks. For instance, the OpenBSD rmuser(1) implementation [0]. [0]: https://github.com/openbsd/src/blob/b69faa6c70c5bfcfdddc6138cd8e0ee18cc15b03/usr.sbin/adduser/rmuser.perl#L143-L151 --- loginutils/deluser.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/loginutils/deluser.c b/loginutils/deluser.c index 56bc7eaa6..633958b9c 100644 --- a/loginutils/deluser.c +++ b/loginutils/deluser.c @@ -99,8 +99,16 @@ int deluser_main(int argc, char **argv) pfile = bb_path_passwd_file; if (ENABLE_FEATURE_SHADOWPASSWDS) sfile = bb_path_shadow_file; - if (opt_delhome) - remove_file(pw->pw_dir, FILEUTILS_RECUR); + if (opt_delhome) { + struct stat st; + + /* Make sure home is an actual directory before +* removing it (e.g. users with /dev/null as home) */ + if (stat(pw->pw_dir, )) + bb_error_msg_and_die("can't stat '%s'", pw->pw_dir); + if (S_ISDIR(st.st_mode)) + remove_file(pw->pw_dir, FILEUTILS_RECUR); + } } else { struct group *gr; do_delgroup: ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] cpio: don't complain about unsafe symlink target when using -p
From: Sören Tempel <soeren+...@soeren-tempel.net> Without this patch busybox complains about unsafe symlink target even when you are just copying files to a directory since OPT_EXTRACT is set by the main function when -p is being used. This patch is kind of ugly. An alternative solution would be not calling get_header_cpio when -p is being used. --- archival/cpio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/archival/cpio.c b/archival/cpio.c index 1d6cbd1e2..9aa23ece1 100644 --- a/archival/cpio.c +++ b/archival/cpio.c @@ -449,6 +449,7 @@ int cpio_main(int argc UNUSED_PARAM, char **argv) xmove_fd(pp.rd, STDIN_FILENO); //opt &= ~OPT_PASSTHROUGH; opt |= OPT_EXTRACT; + setenv("EXTRACT_UNSAFE_SYMLINKS", "1", 1); goto skip; } /* -o */ -- 2.16.2 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox