[PATCH v2] umount: Implement -O option to unmount by mount options

2023-03-03 Thread soeren
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

2022-06-19 Thread soeren
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

2022-06-01 Thread soeren
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

2022-06-01 Thread soeren
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

2022-02-27 Thread soeren
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

2022-02-08 Thread soeren
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

2022-01-29 Thread soeren
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

2021-12-29 Thread soeren
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

2021-11-21 Thread soeren
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

2021-11-20 Thread soeren
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

2021-11-17 Thread soeren
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

2021-11-17 Thread soeren
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

2021-07-17 Thread soeren
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

2021-05-23 Thread soeren
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

2020-06-09 Thread soeren
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

2020-06-09 Thread soeren
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

2018-03-12 Thread soeren
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