[PATCH] awk: fix ternary operator and precedence of =

[Natanael Copa]

Adjust the = precedence test to match behavior of gawk, mawk and
FreeBSD.  awk 'BEGIN {print v=3==3; print v}' should print two '1'.

To fix this, and to unbreak the ternary conditional operator, we restore
the precedence of = in the token list, but override this with a lower
priority when the assignment is on the right side of a compare.

This fixes commit 0256e00a9d07 (awk: fix precedence of = relative to ==)

Signed-off-by: Natanael Copa 
---
 editors/awk.c   | 18 ++
 testsuite/awk.tests |  9 +++--
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/editors/awk.c b/editors/awk.c
index 0981c6735..cf46b4e5b 100644
--- a/editors/awk.c
+++ b/editors/awk.c
@@ -442,9 +442,10 @@ static const uint32_t tokeninfo[] ALIGN4 = {
 #define TI_PREINC (OC_UNARY|xV|P(9)|'P')
 #define TI_PREDEC (OC_UNARY|xV|P(9)|'M')
TI_PREINC,   TI_PREDEC,   OC_FIELD|xV|P(5),
-   OC_COMPARE|VV|P(39)|5,   OC_MOVE|VV|P(38),
OC_REPLACE|NV|P(38)|'+', OC_REPLACE|NV|P(38)|'-',
-   OC_REPLACE|NV|P(38)|'*', OC_REPLACE|NV|P(38)|'/', 
OC_REPLACE|NV|P(38)|'%', OC_REPLACE|NV|P(38)|'&',
-   OC_BINARY|NV|P(29)|'+',  OC_BINARY|NV|P(29)|'-',  
OC_REPLACE|NV|P(38)|'&', OC_BINARY|NV|P(15)|'&',
+#define TI_ASSIGN (OC_MOVE|VV|P(74))
+   OC_COMPARE|VV|P(39)|5,   TI_ASSIGN,   
OC_REPLACE|NV|P(74)|'+', OC_REPLACE|NV|P(74)|'-',
+   OC_REPLACE|NV|P(74)|'*', OC_REPLACE|NV|P(74)|'/', 
OC_REPLACE|NV|P(74)|'%', OC_REPLACE|NV|P(74)|'&',
+   OC_BINARY|NV|P(29)|'+',  OC_BINARY|NV|P(29)|'-',  
OC_REPLACE|NV|P(74)|'&', OC_BINARY|NV|P(15)|'&',
OC_BINARY|NV|P(25)|'/',  OC_BINARY|NV|P(25)|'%',  
OC_BINARY|NV|P(15)|'&',  OC_BINARY|NV|P(25)|'*',
OC_COMPARE|VV|P(39)|4,   OC_COMPARE|VV|P(39)|3,   
OC_COMPARE|VV|P(39)|0,   OC_COMPARE|VV|P(39)|1,
 #define TI_LESS (OC_COMPARE|VV|P(39)|2)
@@ -1390,11 +1391,19 @@ static node *parse_expr(uint32_t term_tc)
continue;
}
if (tc & (TS_BINOP | TC_UOPPOST)) {
+   int prio;
debug_printf_parse("%s: TS_BINOP | TC_UOPPOST tc:%x\n", 
__func__, tc);
/* for binary and postfix-unary operators, jump back 
over
 * previous operators with higher priority */
vn = cn;
-   while (((t_info & PRIMASK) > (vn->a.n->info & PRIMASK2))
+   /* Let assignment get higher priority when used on right
+* side in compare. i.e: 2==v=3 */
+   if (t_info == TI_ASSIGN && (vn->a.n->info & OPCLSMASK) 
== OC_COMPARE) {
+   prio = PRECEDENCE(38);
+   } else {
+   prio = (t_info & PRIMASK);
+   }
+   while ((prio > (vn->a.n->info & PRIMASK2))
|| (t_info == vn->info && t_info == TI_COLON)
) {
vn = vn->a.n;
@@ -1426,6 +1435,7 @@ static node *parse_expr(uint32_t term_tc)
if ((vn->info & OPCLSMASK) != OC_VAR
 && (vn->info & OPCLSMASK) != OC_FNARG
 && (vn->info & OPCLSMASK) != OC_FIELD
+&& (vn->info & OPCLSMASK) != OC_COMPARE
) {
syntax_error(EMSG_UNEXP_TOKEN); 
/* no. bad */
}
diff --git a/testsuite/awk.tests b/testsuite/awk.tests
index 063084a1c..d3ca476a7 100755
--- a/testsuite/awk.tests
+++ b/testsuite/awk.tests
@@ -547,9 +547,14 @@ testing 'awk does not split on CR (char 13)' \
'word1 word2 word3\r word2 word3\r\n' \
'' 'word1 word2 word3\r'
 
-testing "awk = has higher precedence than == (despite what gawk manpage 
claims)" \
+testing "awk = has higher precedence than == on right side" \
"awk 'BEGIN { v=1; print 2==v; print 2==v=2; print v; print v=3==3; 
print v}'" \
-   '0\n1\n2\n1\n3\n' \
+   '0\n1\n2\n1\n1\n' \
+   '' ''
+
+testing 'awk ternary precedence' \
+   "awk 'BEGIN { a = 0 ? \"yes\": \"no\"; print a }'" \
+   'no\n' \
'' ''
 
 sq="'"
-- 
2.45.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] tests: add test for ternary precedence

[Natanael Copa]

This tests a regression introduced by commit 0256e00a9d07 (awk: fix
precedence of = relative to ==).

Signed-off-by: Natanael Copa 
---
 testsuite/awk.tests | 5 +
 1 file changed, 5 insertions(+)

diff --git a/testsuite/awk.tests b/testsuite/awk.tests
index 063084a1c..a56232d02 100755
--- a/testsuite/awk.tests
+++ b/testsuite/awk.tests
@@ -552,6 +552,11 @@ testing "awk = has higher precedence than == (despite what 
gawk manpage claims)"
'0\n1\n2\n1\n3\n' \
'' ''
 
+testing 'awk ternary precedence' \
+   "awk 'BEGIN { a = 0 ? \"yes\": \"no\"; print a }'" \
+   'no\n' \
+   '' ''
+
 sq="'"
 testing 'awk gensub backslashes \' \
'awk '$sq'BEGIN { s="\\"; print "s=" s; print gensub("a", s, "g", 
"a|a") }'$sq \
-- 
2.45.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] awk: fix use after free (CVE-2023-42363)

[Natanael Copa]

On Tue, 21 May 2024 08:46:46 +0200
Natanael Copa  wrote:

> > Current git master awk is also broken.  
> 
> A reproducer:
> 
> busybox awk 'BEGIN { a = 0 ? "yes": "no"; print a}'
> 
> Prints 0 instead of "no".
> 
> Looks like awk treats it as: (a = 0) ? "yes": "no"


I think commit 0256e00a9d07 (awk: fix precedence of = relative to ==)
may be wrong? At least it is different from other awk implementations:

ncopa-desktop:~/src/busybox/build (master)$ ./busybox awk 'BEGIN { print 
v=3==3; print v}'
1
3
ncopa-desktop:~/src/busybox/build (master)$ gawk 'BEGIN { print v=3==3; print 
v}'
1
1
ncopa-desktop:~/src/busybox/build (master)$ mawk 'BEGIN { print v=3==3; print 
v}'
1
1
ncopa-desktop:~/src/busybox/build (master)$ nawk 'BEGIN { print v=3==3; print 
v}'
nawk: syntax error at source line 1
 context is
BEGIN { print >>>  v=3== <<< 
nawk: illegal statement at source line 1
nawk: illegal statement at source line 1
ncopa-desktop:~/src/busybox/build (master)$ nawk 'BEGIN { print (v=3==3); print 
v}'
1
1
ncopa-desktop:~/src/busybox/build (master)$ ./busybox awk 'BEGIN { print 
(v=3==3); print v}'
1
3

And on FreeBSD:

ncopa@fbsd13:~ $ awk 'BEGIN { print v=3==3; print v}'
awk: syntax error at source line 1
 context is
BEGIN { print >>>  v=3== <<< 
awk: illegal statement at source line 1


The awk.tests claims:
> "awk = has higher precedence than == (despite what gawk manpage claims)"

Denys, from where did you get that "= has higher precedence than =="? I
haven't found any other awk implementation where this is true. 

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] awk: fix use after free (CVE-2023-42363)

[Natanael Copa]

On Tue, 21 May 2024 07:43:03 +0200
Natanael Copa  wrote:

> Hi again,
> 
> On Mon, 20 May 2024 22:52:44 +0200
> Natanael Copa  wrote:
> 
> > On Mon, 20 May 2024 17:55:28 +0200
> > Natanael Copa  wrote:
> >   
> > > Fixes https://bugs.busybox.net/show_bug.cgi?id=15865
> > 
> > I also found out that CVE-2023-42364 and CVE-2023-42365 are fixed with
> > commit 0256e00a9d07 (awk: fix precedence of = relative to ==).  
> 
> We discovered that this specific commit also breaks autotools test TAP output.
> https://www.gnu.org/software/automake/manual/html_node/Use-TAP-with-the-Automake-test-harness.html
> 
> This was discovered when building 
> https://lttng.org/files/lttng-ust/lttng-ust-2.13.8.tar.bz2
> run: ./configure && make -j$(nproc) && make AWK="/path/to/busybox awk" check
> 
> Current git master awk is also broken.

A reproducer:

busybox awk 'BEGIN { a = 0 ? "yes": "no"; print a}'

Prints 0 instead of "no".

Looks like awk treats it as: (a = 0) ? "yes": "no"

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] awk: fix use after free (CVE-2023-42363)

[Natanael Copa]

Hi again,

On Mon, 20 May 2024 22:52:44 +0200
Natanael Copa  wrote:

> On Mon, 20 May 2024 17:55:28 +0200
> Natanael Copa  wrote:
> 
> > Fixes https://bugs.busybox.net/show_bug.cgi?id=15865  
> 
> I also found out that CVE-2023-42364 and CVE-2023-42365 are fixed with
> commit 0256e00a9d07 (awk: fix precedence of = relative to ==).

We discovered that this specific commit also breaks autotools test TAP output.
https://www.gnu.org/software/automake/manual/html_node/Use-TAP-with-the-Automake-test-harness.html

This was discovered when building 
https://lttng.org/files/lttng-ust/lttng-ust-2.13.8.tar.bz2
run: ./configure && make -j$(nproc) && make AWK="/path/to/busybox awk" check

Current git master awk is also broken.

-nc

> 
> See: https://bugs.busybox.net/show_bug.cgi?id=15871#c5
> 
> It would be nice if those two were backported to 1_36_stable.



> 
> Thanks!
> 
> -nc
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] awk: fix use after free (CVE-2023-42363)

[Natanael Copa]

On Mon, 20 May 2024 17:55:28 +0200
Natanael Copa  wrote:

> Fixes https://bugs.busybox.net/show_bug.cgi?id=15865

I also found out that CVE-2023-42364 and CVE-2023-42365 are fixed with
commit 0256e00a9d07 (awk: fix precedence of = relative to ==).

See: https://bugs.busybox.net/show_bug.cgi?id=15871#c5

It would be nice if those two were backported to 1_36_stable.

Thanks!

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] awk: fix use after free (CVE-2023-42363)

[Natanael Copa]

Fixes https://bugs.busybox.net/show_bug.cgi?id=15865

Signed-off-by: Natanael Copa 
---
 editors/awk.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/editors/awk.c b/editors/awk.c
index 0981c6735..122376935 100644
--- a/editors/awk.c
+++ b/editors/awk.c
@@ -2981,10 +2981,6 @@ static var *evaluate(node *op, var *res)
/* yes, remember where Fields[] is */
old_Fields_ptr = Fields;
}
-   if (opinfo & OF_STR1) {
-   L.s = getvar_s(L.v);
-   debug_printf_eval("L.s:'%s'\n", L.s);
-   }
if (opinfo & OF_NUM1) {
L_d = getvar_i(L.v);
debug_printf_eval("L_d:%f\n", L_d);
@@ -3014,6 +3010,14 @@ static var *evaluate(node *op, var *res)
}
}
 
+   /* Must get L.s after R.v is evaluated in case it realloc's L.v.
+* eg: x = (v = "abc",  gsub("b", "X", v));
+*/
+   if ((opinfo & OF_RES1) && (opinfo & OF_STR1)) {
+   L.s = getvar_s(L.v);
+   debug_printf_eval("L.s:'%s'\n", L.s);
+   }
+
debug_printf_eval("switch(0x%x)\n", XC(opinfo & OPCLSMASK));
switch (XC(opinfo & OPCLSMASK)) {
 
-- 
2.45.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] wget: add header Accept: */*

[Natanael Copa]

Some servers (like https://netfilter.org) returns failure if the Accept
header is missing. Both GNU wget and curl adds this header, so make
busybox do the same.

fixes: https://bugs.busybox.net/show_bug.cgi?id=15982

function old new   delta
wget_main   31203144 +24
.rodata79296   79310 +14
wget_user_headers 76  84  +8
--
(add/remove: 0/0 grow/shrink: 3/0 up/down: 46/0)   Total: 46 bytes
   textdata bss dec hex filename
 825278   142602008  841546   cd74a busybox_old
 825324   142602008  841592   cd778 busybox_unstripped
---
 networking/wget.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/networking/wget.c b/networking/wget.c
index 199ddd4da..48486fdee 100644
--- a/networking/wget.c
+++ b/networking/wget.c
@@ -212,14 +212,16 @@ enum {
HDR_USER_AGENT= (1<<1),
HDR_RANGE = (1<<2),
HDR_CONTENT_TYPE  = (1<<3),
-   HDR_AUTH  = (1<<4) * ENABLE_FEATURE_WGET_AUTHENTICATION,
-   HDR_PROXY_AUTH= (1<<5) * ENABLE_FEATURE_WGET_AUTHENTICATION,
+   HDR_ACCEPT= (1<<4),
+   HDR_AUTH  = (1<<5) * ENABLE_FEATURE_WGET_AUTHENTICATION,
+   HDR_PROXY_AUTH= (1<<6) * ENABLE_FEATURE_WGET_AUTHENTICATION,
 };
 static const char wget_user_headers[] ALIGN1 =
"Host:\0"
"User-Agent:\0"
"Range:\0"
"Content-Type:\0"
+   "Accept:\0"
 # if ENABLE_FEATURE_WGET_AUTHENTICATION
"Authorization:\0"
"Proxy-Authorization:\0"
@@ -229,6 +231,7 @@ static const char wget_user_headers[] ALIGN1 =
 # define USR_HEADER_USER_AGENT   (G.user_headers & HDR_USER_AGENT)
 # define USR_HEADER_RANGE(G.user_headers & HDR_RANGE)
 # define USR_HEADER_CONTENT_TYPE (G.user_headers & HDR_CONTENT_TYPE)
+# define USR_HEADER_ACCEPT   (G.user_headers & HDR_ACCEPT)
 # define USR_HEADER_AUTH (G.user_headers & HDR_AUTH)
 # define USR_HEADER_PROXY_AUTH   (G.user_headers & HDR_PROXY_AUTH)
 #else /* No long options, no user-headers :( */
@@ -236,6 +239,7 @@ static const char wget_user_headers[] ALIGN1 =
 # define USR_HEADER_USER_AGENT   0
 # define USR_HEADER_RANGE0
 # define USR_HEADER_CONTENT_TYPE 0
+# define USR_HEADER_ACCEPT   0
 # define USR_HEADER_AUTH 0
 # define USR_HEADER_PROXY_AUTH   0
 #endif
@@ -1232,6 +1236,8 @@ static void download_one_url(const char *url)
SENDFMT(sfp, "Host: %s\r\n", target.host);
if (!USR_HEADER_USER_AGENT)
SENDFMT(sfp, "User-Agent: %s\r\n", G.user_agent);
+   if (!USR_HEADER_ACCEPT)
+   SENDFMT(sfp, "Accept: */*\r\n");
 
/* Ask server to close the connection as soon as we are done
 * (IOW: we do not intend to send more requests)
-- 
2.44.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] time: fix max resident set size unit

[Natanael Copa]

The ru_maxrss is already in Kbytes and not pages.

function old new   delta
time_main   11951190  -5
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-5)   Total: -5 bytes
   textdata bss dec hex filename
 828010   142682008  844286   ce1fe busybox_old
 828005   142682008  844281   ce1f9 busybox_unstripped

fixes: https://bugs.busybox.net/show_bug.cgi?id=15751
---

Good catch!

Thanks!

 miscutils/time.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/miscutils/time.c b/miscutils/time.c
index 5a8fa4c0b..b90b582b0 100644
--- a/miscutils/time.c
+++ b/miscutils/time.c
@@ -281,7 +281,7 @@ static void summarize(const char *fmt, char **command, 
resource_t *resp)
 ptok(pagesize, (UL) 
resp->ru.ru_ixrss)) / cpu_ticks);
break;
case 'M':   /* Maximum resident set size.  */
-   printf("%lu", ptok(pagesize, (UL) 
resp->ru.ru_maxrss));
+   printf("%lu", (UL) resp->ru.ru_maxrss);
break;
case 'O':   /* Outputs.  */
printf("%lu", resp->ru.ru_oublock);
-- 
2.43.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH resend] sed: check errors writing file with sed -i

[Natanael Copa]

Hi!

Any change to get this merged?

We have used it in alpine for a while, and so far no issues.

Thanks!

-nc

On Tue, 19 Sep 2023 17:11:02 +0900
Dominique Martinet  wrote:

> From: Dominique Martinet 
> 
> sed would currently not error if write failed when modifying a file.
> 
> This can be reproduced with the following 'script':
> $ sudo mount -t tmpfs tmpfs -o size=1M /tmp/m
> $ sudo chmod 777 /tmp/m
> $ echo foo > /tmp/m/foo
> $ dd if=/dev/zero of=/tmp/m/fill bs=4k
> dd: error writing '/tmp/m/fill': No space left on device
> 256+0 records in
> 255+0 records out
> 1044480 bytes (1.0 MB, 1020 KiB) copied, 0.00234567 s, 445 MB/s
> $ busybox sed -i -e 's/.*/bar/' /tmp/m/foo
> $ echo $?
> 0
> $ cat /tmp/m/foo
> 
> 
> new behaviour:
> $ echo foo > /tmp/m/foo
> $ ./busybox sed -i -e 's/.*/bar/' /tmp/m/foo
> sed: write error
> $ echo $?
> 4
> $ cat /tmp/m/foo
> foo
> 
> function old new   delta
> sed_main 754 801 +47
> --
> (add/remove: 0/0 grow/shrink: 1/0 up/down: 47/0)   Total: 47 bytes
>text  data bss dec hex filename
>   75727  25101552   79789   137ad busybox_old
>   75774  25101552   79836   137dc busybox_unstripped
> 
> Signed-off-by: Dominique Martinet 
> ---
> Resending this patch again as it doesn't seem to have been applied.
> 
> FWIW it's still applied on alpine builds (since Nov last year); would be
> great to see this merged so I can forget about it :)
> 
>  editors/sed.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/editors/sed.c b/editors/sed.c
> index 00dde60be329..6179c5e80958 100644
> --- a/editors/sed.c
> +++ b/editors/sed.c
> @@ -1648,6 +1648,11 @@ int sed_main(int argc UNUSED_PARAM, char **argv)
>   fchown(nonstdoutfd, statbuf.st_uid, statbuf.st_gid);
>  
>   process_files();
> + fflush(G.nonstdout);
> + if (ferror(G.nonstdout)) {
> + xfunc_error_retval = 4;  /* It's what gnu sed 
> exits with... */
> + bb_simple_error_msg_and_die(bb_msg_write_error);
> + }
>   fclose(G.nonstdout);
>   G.nonstdout = stdout;
>  

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] find: fix -xdev -depth (and -delete)

[Natanael Copa]

Hi!

I think this bug looks pretty bad, as it deletes stuff unintentionally.

And appears to reduce size, so I'd think this would be nice!

Denys, what do you think?

Thanks!

-nc

On Tue, 19 Sep 2023 17:17:47 +0900
Dominique Martinet  wrote:

> From: Dominique Martinet 
> 
> find -xdev with -depth would check for same_fs after the subdirectory
> has been processed (because the check is done in the file/dir action,
> which is evaluated too late in the -depth case)
> This renders `find -xdev -delete` useless, as reported in 2012 here:
> https://bugs.busybox.net/show_bug.cgi?id=5756
> 
> The bug report suggested adding an extra hook, which would be required
> if we were to keep the current xdev approach that allows all filesystems
> given in argument, but GNU findutils and OpenBSD find actually stop on
> the first filesystem boundary e.g. for the following tree:
> 
> $ find test -exec stat --format "%d %n"  {} +
> 27 test
> 27 test/file
> 59 test/tmpfs
> 27 test/tmpfs/bind
> 27 test/tmpfs/bind/file
> 59 test/tmpfs/file
> (Where 'test/tmpfs' is a tmpfs, and 'test/tmpfs/bind' is a bind mount
> to a neighboring directory in the same filesystem as 'test' -- also
> tested with a symlink and -follow for openbsd which has no bind mount)
> 
> Then `find test test/tmpfs -xdev` does not print test/tmpfs/bind/file.
> 
> This makes the implementation much simpler (although it's a bit ugly to
> carry the parent st_dev as an argument to the function) and smaller
> code, and would allow for easy addition of rm/cp --one-file-system if
> we want to do that later.
> 
> function old new   delta
> recursive_action1398 425 +27
> parse_params16601670 +10
> recursive_action  70  72  +2
> fileAction   216 127 -89
> find_main540 442 -98
> --
> (add/remove: 0/0 grow/shrink: 3/2 up/down: 39/-187)  Total: -148 bytes
>text  data bss dec hex filename
>   75774  25101552   79836   137dc busybox_old
>   75626  25101552   79688   13748 busybox_unstripped
> ---
> This has biten me recently and seems to be a known issue for a while.
> 
> I don't recall seeing any patch for this since I've been on the list so
> I took a quick stab, please take a look when you have time.
> 
> file system loops aside, this can be tested on linux with the following
> "script":
> ++
> cd $(mktemp -d bb-find-xdev.XX) || exit
> mkdir tmpfs
> mount -t tmpfs tmpfs tmpfs
> mkdir -p a/b/c
> touch file tmpfs/file a/b/c/file
> find $PWD -xdev -delete
> # ^ errors about tmpfs busy and pwd not empty,
> # but tmpfs/file is left intact and everyting else is gone
> umount tmpfs
> find $PWD -xdev -delete
> +
> 
>  findutils/find.c | 44 ++--
>  include/libbb.h  |  1 +
>  libbb/recursive_action.c | 13 +---
>  3 files changed, 13 insertions(+), 45 deletions(-)
> 
> diff --git a/findutils/find.c b/findutils/find.c
> index 31c9969886f6..a4a6bbc2df91 100644
> --- a/findutils/find.c
> +++ b/findutils/find.c
> @@ -501,7 +501,6 @@ struct globals {
>  #endif
>   action ***actions;
>   smallint need_print;
> - smallint xdev_on;
>   smalluint exitstatus;
>   recurse_flags_t recurse_flags;
>   IF_FEATURE_FIND_EXEC_PLUS(unsigned max_argv_len;)
> @@ -1015,26 +1014,10 @@ static int FAST_FUNC fileAction(
>   struct stat *statbuf)
>  {
>   int r;
> - int same_fs = 1;
> -
> -#if ENABLE_FEATURE_FIND_XDEV
> - if (S_ISDIR(statbuf->st_mode) && G.xdev_count) {
> - int i;
> - for (i = 0; i < G.xdev_count; i++) {
> - if (G.xdev_dev[i] == statbuf->st_dev)
> - goto found;
> - }
> - //bb_error_msg("'%s': not same fs", fileName);
> - same_fs = 0;
> - found: ;
> - }
> -#endif
>  
>  #if ENABLE_FEATURE_FIND_MAXDEPTH
>   if (state->depth < G.minmaxdepth[0]) {
> - if (same_fs)
> - return TRUE; /* skip this, continue recursing */
> - return SKIP; /* stop recursing */
> + return TRUE; /* skip this, continue recursing */
>   }
>   if (state->depth > G.minmaxdepth[1])
>   return SKIP; /* stop recursing */
> @@ -1051,11 +1034,6 @@ static int FAST_FUNC fileAction(
>   return SKIP;
>   }
>  #endif
> - /* -xdev stops on mountpoints, but AFTER mountpoit itself
> -  * is processed as usual */
> - if (!same_fs) {
> - return SKIP;
> - }
>  
>   /* Cannot return 0: our caller, recursive_action(),
>* will perror() and skip dirs (if 

Re: sha256sum Illegal instruction on musl amd64

[Natanael Copa]

On Wed, 29 Mar 2023 15:46:58 +0200
Denys Vlasenko  wrote:

> > > it's caused by having a cpu with AVX512 (the github runners do) but not 
> > > sha_ni,
> > > and the code that checks it is broken and misdetects sha_ni support when 
> > > avx512
> > > exists. the github runners don't have sha_ni, so it breaks exactly there. 
> > >  
> >
> > The code is:
> >
> > if (!shaNI) {
> > unsigned eax = 7, ebx = ebx, ecx = 0, edx = edx;
> > cpuid(, , , );
> > shaNI = ((ebx >> 29) << 1) - 1;
> > }
> > if (shaNI > 0)
> > ctx->process_block = sha1_process_block64_shaNI;
> >
> > If ebx's bit 29 is set, then shaNI = 1. If it is clear, then shaNI = -1.  
> 
> ...nd this is not true! The bits 30,31 also will affect the result,
> need to be masked out! For example this way:
> 
>shaNI = ((ebx >> 28) & 2) - 1;
> 
> Sorry...

Thank you for the fix. I can confirm it solves it.
https://github.com/ncopa/busybox/actions/runs/4568076877/jobs/8062671513

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: sha256sum Illegal instruction on musl amd64

[Natanael Copa]

On Wed, 29 Mar 2023 15:07:49 +0200
Denys Vlasenko  wrote:

> On Wed, Mar 1, 2023 at 12:01*PM alice  wrote:
> > On Tue Feb 28, 2023 at 11:17 PM CET,  wrote:  
> > > I'm having an intermittent issue with "BusyBox v1.36.0 (2023-01-03 
> > > 22:49:12
> > > UTC)" (the one from the Docker image busybox:musl) when running on amd64 
> > > GitHub
> > > actions runner VMs (azure).
> > >
> > > When I use sha256sum it is getting terminated with SIGILL, Illegal
> > > instruction. The issue is hard to reproduce but I have a GitHub actions 
> > > CI/CD
> > > job that I can re-run repeatedly (no changes to code, environment, data 
> > > input,
> > > etc) that will occasionally have the issue. I managed to capture a core 
> > > dump.  
> >
> > this was also reported in 
> > http://lists.busybox.net/pipermail/busybox/2023-January/090113.html
> >
> > it's caused by having a cpu with AVX512 (the github runners do) but not 
> > sha_ni,
> > and the code that checks it is broken and misdetects sha_ni support when 
> > avx512
> > exists. the github runners don't have sha_ni, so it breaks exactly there.  
> 
> The code is:
> 
> if (!shaNI) {
> unsigned eax = 7, ebx = ebx, ecx = 0, edx = edx;
> cpuid(, , , );
> shaNI = ((ebx >> 29) << 1) - 1;
> }
> if (shaNI > 0)
> ctx->process_block = sha1_process_block64_shaNI;
> 
> If ebx's bit 29 is set, then shaNI = 1. If it is clear, then shaNI = -1.
> 
> I checked
> "Intel® 64 and IA-32 Architectures Software Developer*s Manual
> Combined Volumes: 1, 2A, 2B, 2C, 2D, 3A, 3B, 3C, 3D and 4"
> 
> to verify that CPUID(EAX=7, ECX=0) and bit position (EBX.29)
> is the correct check for SHA-NI.
> 
> I double-checked it in Linux kernel source.
> 
> I checked disassembly and it correctly sets up registers for CPUID.
> 
> Can someone tell me on what CPU this does not work?

Github runners. I created a job where it fails. See here:
You have output of cpuinfo here:
https://github.com/ncopa/busybox/actions/runs/3947969708/jobs/6757415539#step:4:1

I also tried to fix it with this change, but failed:
https://github.com/ncopa/busybox/commit/e4ad5e7f2fed8e36d0779d918052169fe9a0bb95

This was based on the example function `int CheckForIntelShaExtensions()` in 
https://www.intel.com/content/www/us/en/developer/articles/technical/intel-sha-extensions.html

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: sha256sum Illegal instruction on musl amd64

[Natanael Copa]

Hi,

On Wed, 01 Mar 2023 11:55:16 +0100
"alice"  wrote:

> On Tue Feb 28, 2023 at 11:17 PM CET,  wrote:
> > Hi,
> >
> > I'm having an intermittent issue with "BusyBox v1.36.0 (2023-01-03 22:49:12
> > UTC)" (the one from the Docker image busybox:musl) when running on amd64 
> > GitHub
> > actions runner VMs (azure).
> >
> > When I use sha256sum it is getting terminated with SIGILL, Illegal
> > instruction. The issue is hard to reproduce but I have a GitHub actions 
> > CI/CD
> > job that I can re-run repeatedly (no changes to code, environment, data 
> > input,
> > etc) that will occasionally have the issue. I managed to capture a core 
> > dump.  
> 
> this was also reported in 
> http://lists.busybox.net/pipermail/busybox/2023-January/090113.html
> 
> it's caused by having a cpu with AVX512 (the github runners do) but not 
> sha_ni,
> and the code that checks it is broken and misdetects sha_ni support when 
> avx512
> exists. the github runners don't have sha_ni, so it breaks exactly there.
> 
> quite the rare combo :)
> 
> i feel this was reported before, but apparently not outside an irc channel..

I did try to create a fix for it:
https://github.com/ncopa/busybox/commit/e4ad5e7f2fed8e36d0779d918052169fe9a0bb95

But it didn't work. I was unable to create a proper core dump and sort
of gave up. In Alpine we have simply disabled the HWACCEL as it is
broken.

Thanks!
-nc

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

sha_ni detection for sha hwaccel is broken

[Natanael Copa]

Hi,

We have an issue where busybox sha1sum and/or sha256sum is broken when
HWACCEL is enabled. This has been observed on github runners only so far.

I cloned busybox repo and added github runner that build only sha1sum
and sha256sum with hwaccel enabled. I added debug printf statements to
see if shaNI was enabled or not.

What happened was that sometimes shaNI is enabled, and sometimes it is
not.

You have example of this here:

https://github.com/ncopa/busybox/actions/runs/3948518291/jobs/6758600595

As you see, when sha256sum runs, shaNI is -1, but later, using the same
binary shaNI becomes 11 and ends up with illegal instruction.

Something is off, but I'm not sure what.

Disabling hwaccel makes it all go away.

I have not been able to reproduce this on my i9. It has only been
observed on github runners so far.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: BusyBox 1.36.0 regression: Segfaults on i386 musl libc

[Natanael Copa]

On Sun, 08 Jan 2023 20:22:31 +0100
Sören Tempel  wrote:

> Investigated this further. The problem is a text relocation created by
> the hash_md5_sha256_x86-32_shaNI.S file. When compiling BusyBox with
> LDFLAGS=-Wl,-z,text one is warned about the following relocation by gcc:
> 
>   
> /usr/lib/gcc/i586-alpine-linux-musl/12.2.1/../../../../i586-alpine-linux-musl/bin/ld:
>  libbb/lib.a(hash_md5_sha_x86-32_shaNI.o): warning: relocation in read-only 
> section `.text.sha1_process_block64_shaNI'
>   
> /usr/lib/gcc/i586-alpine-linux-musl/12.2.1/../../../../i586-alpine-linux-musl/bin/ld:
>  read-only segment has dynamic relocations
> 

Also scanelf confirms that there are textrels in 3 places:

$ scanelf --textrels busybox_unstripped
 TYPE   TEXTRELS FILE 
  busybox_unstripped: (memory/data?) [r_offset=0x816E] r_type=8 in (optimized 
out: previous sha256_process_block64_shaNI) [closest_prev_sym=0x8158]
  busybox_unstripped: (memory/data?) [r_offset=0x8173] r_type=8 in (optimized 
out: previous sha256_process_block64_shaNI) [closest_prev_sym=0x8158]
  busybox_unstripped: (memory/data?) [r_offset=0x8415] r_type=8 in (optimized 
out: previous sha1_process_block64_shaNI) [closest_prev_sym=0x8400]
ET_DYN  busybox_unstripped 

> The Linux Kernel, from which the assembly was copied, does addressing
> relative to the %pic register to avoid this relocation it seems [1]:
> 
>   movdqa PSHUFFLE_BYTE_FLIP_MASK(%rip), SHUF_MASK
> 
> However, the %rip register is AFAIK not available for i386 and since I
> am personally not an x86 wizard I have no idea how to best rewrite this
> code in a way that it doesn't require dynamic relocations.

For sha1 we can (ab)use the stack for the PSHUFFLE_BYTE_FLIP_MASK data when we 
do position independent code (PIC):

diff --git a/libbb/hash_md5_sha_x86-32_shaNI.S 
b/libbb/hash_md5_sha_x86-32_shaNI.S
index 7455a29f0..4c3d1ea88 100644
--- a/libbb/hash_md5_sha_x86-32_shaNI.S
+++ b/libbb/hash_md5_sha_x86-32_shaNI.S
@@ -49,7 +49,16 @@ sha1_process_block64_shaNI:
pinsrd  $3, 76+4*4(%eax), E0# load to uppermost 32-bit word
shuf128_32  $0x1B, ABCD, ABCD   # DCBA -> ABCD
 
+#ifdef __PIC__
+   pushl   0x0c0d0e0f
+   pushl   0x08090a0b
+   pushl   0x04050607
+   pushl   0x00010203
+   mova128 (%esp), %xmm7
+   addl16, %esp
+#else
mova128 PSHUFFLE_BYTE_FLIP_MASK, %xmm7
+#endif
 
movu128 0*16(%eax), MSG0
pshufb  %xmm7, MSG0
@@ -225,10 +234,11 @@ sha1_process_block64_shaNI:
 
ret
.size   sha1_process_block64_shaNI, .-sha1_process_block64_shaNI
-
+#ifndef __PIC__
.section.rodata.cst16.PSHUFFLE_BYTE_FLIP_MASK, "aM", @progbits, 
16
.balign 16
 PSHUFFLE_BYTE_FLIP_MASK:
.octa   0x000102030405060708090a0b0c0d0e0f
+#endif
 
 #endif


But for the $K256 data we'd need use the global offset table. Not sure exactly 
how to do that.
> 
> [1]: 
> https://github.com/torvalds/linux/blob/94a855111ed9106971ca2617c5d075269e6aefde/arch/x86/crypto/sha1_ni_asm.S#L112
> 
> Sören Tempel  wrote:
> > Hello,
> > 
> > Natanael Copa  wrote:  
> > > diff --git a/libbb/hash_md5_sha.c b/libbb/hash_md5_sha.c
> > > index 880ffab01..d2351d3e6 100644
> > > --- a/libbb/hash_md5_sha.c
> > > +++ b/libbb/hash_md5_sha.c
> > > @@ -17,8 +17,11 @@
> > >  # if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
> > >  static void cpuid(unsigned *eax, unsigned *ebx, unsigned *ecx, unsigned 
> > > *edx)
> > >  {
> > > -   asm ("cpuid"
> > > -   : "=a"(*eax), "=b"(*ebx), "=c"(*ecx), "=d"(*edx)
> > > +   asm volatile (
> > > +   "mov %%ebx, %%esi;" /* save %ebx PIC register */
> > > +   "cpuid;"
> > > +   "xchg %%ebx, %%esi;"
> > > +   : "=a"(*eax), "=S"(*ebx), "=c"(*ecx), "=d"(*edx)
> > > : "0"(*eax),  "1"(*ebx),  "2"(*ecx),  "3"(*edx)
> > > );
> > >  }  
> > 
> > Unfortunately, this does not fix the segfault. Since the segfault occurs
> > in musl's dynamic loader I also don't think that this code is
> > reached/executed. Instead, this seems to be a problem with the symbols
> > of the provided assembly file.
> > 
> > I am currently debugging this on a96ccbefe417aaac6a2ce59c788e01fc0f83902f.
> > If I remove the PSHUFFLE_BYTE_FLIP_MASK definition (and the instruction
> > using it) in hash_md5_sha256_x86-32_shaNI.S from the checkout for this
> > commit then the segfault doesn't occur. So this does definitely seem to
> > be a problem with the hash_md5_sha256_x86-32_shaNI.S assembly file...
> > 
> > Greetings,
> > Sören
> > ___
> > busybox mailing list
> > busybox@busybox.net
> > http://lists.busybox.net/mailman/listinfo/busybox  

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: BusyBox 1.36.0 regression: Segfaults on i386 musl libc

[Natanael Copa]

On Thu, 05 Jan 2023 21:39:09 +0100
Sören Tempel  wrote:

> Hi,
> 
> I am the maintainer of the BusyBox package for Alpine Linux. While
> upgrading that package from 1.35.0 to 1.36.0 I noticed a segfault
> on Alpine x86, on all other architectures BusyBox 1.36.0 builds
> fine and passes the tests. On x86 though it segfaults with any
> command-line argument, for example:
> 
>   $ make defconfig
>   $ make
>   $ gdb --args ./busybox_unstripped
>   (gdb) run
>   Starting program: 
> /home/buildozer/aports/main/busybox/src/build-dynamic/busybox_unstripped
> 
>   Program received signal SIGSEGV, Segmentation fault.
>   0xf7fc24e0 in do_relocs (dso=dso@entry=0xf7ffca20 , 
> rel=0x565578e4, rel_size=8712, stride=2) at ldso/dynlink.c:471
>   471 ldso/dynlink.c: No such file or directory.
>   (gdb) bt
>   #0  0xf7fc24e0 in do_relocs (dso=dso@entry=0xf7ffca20 , 
> rel=0x565578e4, rel_size=8712, stride=2) at ldso/dynlink.c:471
>   #1  0xf7fc263f in reloc_all (p=p@entry=0xf7ffca20 ) at 
> ldso/dynlink.c:1375
>   #2  0xf7fc473e in __dls3 (sp=0xdcf0, auxv=0xdd3c) at 
> ldso/dynlink.c:1974
>   #3  0xf7fc3eab in __dls2 (base=, sp=) at 
> ldso/dynlink.c:1719
>   #4  0xf7fc19c9 in _dlstart () from /lib/ld-musl-i386.so.1
> 
> Looking at the backtrace, it seems that it segfaults in musl's dynamic
> loader. Since BusyBox 1.35.0 worked fine on x86 I bisected this and it
> turns out that this is a regression introduced in commit
> a96ccbefe417aaac6a2ce59c788e01fc0f83902f [1]. If I disable SHA/MD5
> hardware acceleration then BusyBox 1.36.0 builds fine and passes all
> tests on Alpine Linux x86.
> 
> Any idea what particular part of the referenced commit might be causing this?

I believe this happens due to ebx is clobbered which is needed for
position independent code (PIC) on 32 bit x86.

I also wonder if the asm needs to be volatile.

Try something like this:

diff --git a/libbb/hash_md5_sha.c b/libbb/hash_md5_sha.c
index 880ffab01..d2351d3e6 100644
--- a/libbb/hash_md5_sha.c
+++ b/libbb/hash_md5_sha.c
@@ -17,8 +17,11 @@
 # if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
 static void cpuid(unsigned *eax, unsigned *ebx, unsigned *ecx, unsigned *edx)
 {
-   asm ("cpuid"
-   : "=a"(*eax), "=b"(*ebx), "=c"(*ecx), "=d"(*edx)
+   asm volatile (
+   "mov %%ebx, %%esi;" /* save %ebx PIC register */
+   "cpuid;"
+   "xchg %%ebx, %%esi;"
+   : "=a"(*eax), "=S"(*ebx), "=c"(*ecx), "=d"(*edx)
: "0"(*eax),  "1"(*ebx),  "2"(*ecx),  "3"(*edx)
);
 }

> 
> Greetings,
> Sören
> 
> [1]:
> https://git.busybox.net/busybox/commit/?id=a96ccbefe417aaac6a2ce59c788e01fc0f83902f
> ___ busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] more: accept and ignore -e

[Natanael Copa]

Accept and ignore -e which is specified in POSIX.
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/more.html
---
 util-linux/more.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/util-linux/more.c b/util-linux/more.c
index eea69da06..a830dcbc1 100644
--- a/util-linux/more.c
+++ b/util-linux/more.c
@@ -84,11 +84,12 @@ int more_main(int argc UNUSED_PARAM, char **argv)
/* Parse options */
/* Accepted but ignored: */
/* -d   Display help instead of ringing bell */
+   /* -e   Exit immediately after writing the last line */
/* -f   Count logical lines (IOW: long lines are not folded) */
/* -l   Do not pause after any line containing a ^L (form feed) */
/* -s   Squeeze blank lines into one */
/* -u   Suppress underlining */
-   getopt32(argv, "dflsu");
+   getopt32(argv, "deflsu");
argv += optind;
 
/* Another popular pager, most, detects when stdout
-- 
2.38.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2 1/2] awk: fix use after free (CVE-2022-30065)

[Natanael Copa]

Denys,

This fix passes `./runtest awk`. We have used this fix in Alpine Linux
edge (development branch) for a week now and as far I know, there have
been no issues due to this.

Can you please have another look? And push it to the stable branch if
possible.

Thanks!

On Fri, 17 Jun 2022 17:45:34 +0200
Natanael Copa  wrote:

> fixes https://bugs.busybox.net/show_bug.cgi?id=14781
> 
> Signed-off-by: Natanael Copa 
> ---
>  editors/awk.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/editors/awk.c b/editors/awk.c
> index 079d0bde5..728ee8685 100644
> --- a/editors/awk.c
> +++ b/editors/awk.c
> @@ -3128,6 +3128,9 @@ static var *evaluate(node *op, var *res)
>  
>   case XC( OC_MOVE ):
>   debug_printf_eval("MOVE\n");
> + /* make sure that we never return a temp var */
> + if (L.v == TMPVAR0)
> + L.v = res;
>   /* if source is a temporary string, jusk relink it to 
> dest */
>   if (R.v == TMPVAR1
>&& !(R.v->type & VF_NUMBER)

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH v2 1/2] awk: fix use after free (CVE-2022-30065)

[Natanael Copa]

fixes https://bugs.busybox.net/show_bug.cgi?id=14781

Signed-off-by: Natanael Copa 
---
 editors/awk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/editors/awk.c b/editors/awk.c
index 079d0bde5..728ee8685 100644
--- a/editors/awk.c
+++ b/editors/awk.c
@@ -3128,6 +3128,9 @@ static var *evaluate(node *op, var *res)
 
case XC( OC_MOVE ):
debug_printf_eval("MOVE\n");
+   /* make sure that we never return a temp var */
+   if (L.v == TMPVAR0)
+   L.v = res;
/* if source is a temporary string, jusk relink it to 
dest */
if (R.v == TMPVAR1
 && !(R.v->type & VF_NUMBER)
-- 
2.36.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH v2 2/2] awk: add tests for CVE-2022-30065

[Natanael Copa]

Signed-off-by: Natanael Copa 
---
 testsuite/awk.tests | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/testsuite/awk.tests b/testsuite/awk.tests
index 93e25d8c1..6c3a03c37 100755
--- a/testsuite/awk.tests
+++ b/testsuite/awk.tests
@@ -479,4 +479,15 @@ testing 'awk backslash+newline eaten with no trace' \
"Hello world\n" \
'' ''
 
+testing 'awk use-after-free (CVE-2022-30065)' \
+   "awk '\$3i\$3in\$9=\$r||\$9=i6/6-9f'" \
+   "" \
+   "" \
+   ""
+
+testing 'awk assign while test' \
+   "awk '\$1==\$1=\"foo\" {print \$1}'" \
+   "foo\n" \
+   "" \
+   "foo"
 exit $FAILCOUNT
-- 
2.36.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] awk: fix use after free (CVE-2022-30065)

[Natanael Copa]

On Thu, 16 Jun 2022 12:54:56 +0200
Natanael Copa  wrote:

> On Tue, 14 Jun 2022 18:24:54 +0200
> Denys Vlasenko  wrote:
> 
> > On Tue, Jun 14, 2022 at 8:55 AM Natanael Copa  
> > wrote:  
> > > Hi!
> > >
> > > Is there anything else I can do to help fix CVE-2022-30065? I have
> > > created a testcase for the testsuite and proposed a fix, but I'm not
> > > that familiar with awk code so I would appreciate some help with this
> > > before pushing it to thousands (millions?) of users.
> > 
> > cd testsuite && ./runtest awk
> > 
> > fails a lot with this change.  
> 
> Indeed, sorry! I thought I ran it locally but I must have done something 
> wrong when running them here.
> 
> Need to go back to the drawing board...

Ok, so I have made some progress and have another possible fix. I also
separated the test case into a separate commit, so it becomes easier to
test in different branches etc. while working on it.

I also have another (valid?) testcase which passes on gawk and nawk,
but fails with my fix suggestion. I also attach a WIP patch which adds
some debug info of pointer values to make it visible when tmpvars are
allocated and when evaluate() returns the just free'd pointer.

Summary of the attached patches:

0001-awk-fix-use-after-free-CVE-2022-30065.patch:
Potential fix, that prevents the use-after-free. But honestly,
I don't really know what I'm doing and I don't know if this
breaks any valid cases.

0002-awk-test-for-CVE-2022-30065.patch:
The test case for the CVE. gawk and mawk returns syntax error
for this test. This patch can be applied as is, but it will not
pass until we have a proper fix.

0003-awk-add-test-for-setting-1-while-testing-it.patch:
A test case that passes with mawk/gawk but fails with the
proposed fix. I don't know if this is valid syntax or if it is
ok to error out with syntax error.

0004-WIP-debug-awk.patch:
Temp patch that adds extra printf debug info to show what
happens. This patch should *not* be applied upstream, but can
be used in local development git branches to help debug.


With the attached debug print 0004-WIP-debug-awk.patch applied, gives
the following output when running: echo | ./busybox awk '$1$1=0'

# My comments are prefixed with a #
# TIP: do a in browser/reader search for '0x7f52929debb0' to highlight
# where the problematic address is used.

fsrealloc: xrealloc(0, 512)
fsrealloc: Fields=0x7f52929df030..0x7f52929df22f
getvar_i: 0.00
getvar_i: 1.00
entered awk_getline()
returning from awk_getline(): 1
getvar_i: 0.00
getvar_i: 0.00
entered evaluate(op=0x7f52929dff30, res=0x7f5292a77328)
tmpvars=0x7f52929deb60
opinfo:0300 opn:
switch(0x3)
NEWSOURCE
opinfo:0d00 opn:
switch(0xd)
TEST
entered evaluate(op=0x7f52929dd530, res=0x7f5292a772a8)
tmpvars=0x7f52929debb0  # this is where allocation happens
opinfo:4a031f00 opn:
entered evaluate(op=0x7f52929dd470, res=0x7f52929debb0) # the buffer is passed 
here
tmpvars=0x7f52929dd820
opinfo:230f1500 opn:
entered evaluate(op=0x7f52929dffc0, res=0x7f52929dd820)
tmpvars=0x7f52929dd870
opinfo:05021700 opn:
entered evaluate(op=0x7f52929dd410, res=0x7f52929dd890)
tmpvars=0x7f52929dd8c0
opinfo:2700 opn:
switch(0x27)
VAR
returning from evaluate(): res=0x7f52929dd440, tmpvars=0x7f52929dd8c0
switch(0x17)
FIELD
getvar_i: 1.00
returning from evaluate(): res=0x7f52929df030, tmpvars=0x7f52929dd870
opinfo & OF_RES1: L.v:'0x7f52929df030'
L.s:''
entered evaluate(op=0x7f52929dd4a0, res=0x7f52929dd840)
tmpvars=0x7f52929dd910
opinfo:05021700 opn:
entered evaluate(op=0x7f52929dd4d0, res=0x7f52929dd930)
tmpvars=0x7f52929dd960
opinfo:2700 opn:
switch(0x27)
VAR
returning from evaluate(): res=0x7f52929dd500, tmpvars=0x7f52929dd960
switch(0x17)
FIELD
getvar_i: 1.00
returning from evaluate(): res=0x7f52929df030, tmpvars=0x7f52929dd910
R.s:''
switch(0x15)
CONCAT /
COMMA: L.s='', sep='', R.s=''
returning from evaluate(): res=0x7f52929debb0, tmpvars=0x7f52929dd820   # 
address is return value here
opinfo & OF_RES1: L.v:'0x7f52929debb0'  # L.v is from TEST tmpvars here
entered evaluate(op=0x7f52929dd560, res=0x7f52929debd0)
tmpvars=0x7f52929dd820
opinfo:2700 opn:
switch(0x27)
VAR
returning from evaluate(): res=0x7f52929dd590, tmpvars=0x7f52929dd820
switch(0x1f)
MOVE L.v=0x7f52929debb0, res=0x7f5292a772a8 # MOVE passes the reference here
copyvar: number:0.00 string:'(null)'# copyvar here will copy the 
reference to res here
returning from evaluate(): res=0x7f52929debb0, tmpvars=0x7f52929debb0 # res is 
now set to same value as tmpvars which is free'd.
# evaluate() here returns the value it just free'd.
awk: cmd. line:

Re: [PATCH] awk: fix use after free (CVE-2022-30065)

[Natanael Copa]

On Tue, 14 Jun 2022 18:24:54 +0200
Denys Vlasenko  wrote:

> On Tue, Jun 14, 2022 at 8:55 AM Natanael Copa  wrote:
> > Hi!
> >
> > Is there anything else I can do to help fix CVE-2022-30065? I have
> > created a testcase for the testsuite and proposed a fix, but I'm not
> > that familiar with awk code so I would appreciate some help with this
> > before pushing it to thousands (millions?) of users.  
> 
> cd testsuite && ./runtest awk
> 
> fails a lot with this change.

Indeed, sorry! I thought I ran it locally but I must have done something wrong 
when running them here.

Need to go back to the drawing board...

Valgrind also show that those (at least one of those) does not touches
memory it shouldn't. Maybe we should set it to null together with free?

The comment says:
>  //TODO: L.v may be invalid now, set L.v to NULL to catch bugs?
But apparently L.v is not always invalid. How do we know when it is invalid and 
when it is not?

Other ideas how to fix this?

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] awk: fix use after free (CVE-2022-30065)

[Natanael Copa]

Hi!

Is there anything else I can do to help fix CVE-2022-30065? I have
created a testcase for the testsuite and proposed a fix, but I'm not
that familiar with awk code so I would appreciate some help with this
before pushing it to thousands (millions?) of users.

Thanks!

On Tue,  7 Jun 2022 21:56:27 +0200
Natanael Copa  wrote:

> fixes https://bugs.busybox.net/show_bug.cgi?id=14781
> ---
>  editors/awk.c   | 6 --
>  testsuite/awk.tests | 6 ++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/editors/awk.c b/editors/awk.c
> index 079d0bde5..be38289e4 100644
> --- a/editors/awk.c
> +++ b/editors/awk.c
> @@ -2921,8 +2921,8 @@ static var *evaluate(node *op, var *res)
>*/
>   if (opinfo & OF_RES2) {
>   R.v = evaluate(op->r.n, TMPVAR1);
> - //TODO: L.v may be invalid now, set L.v to NULL to 
> catch bugs?
> - //L.v = NULL;
> + // L.v may be invalid now, set L.v to NULL to catch bugs
> + L.v = NULL;
>   if (opinfo & OF_STR2) {
>   R.s = getvar_s(R.v);
>   debug_printf_eval("R.s:'%s'\n", R.s);
> @@ -3128,6 +3128,8 @@ static var *evaluate(node *op, var *res)
>  
>   case XC( OC_MOVE ):
>   debug_printf_eval("MOVE\n");
> + if (L.v == NULL)
> + syntax_error(EMSG_POSSIBLE_ERROR);
>   /* if source is a temporary string, jusk relink it to 
> dest */
>   if (R.v == TMPVAR1
>&& !(R.v->type & VF_NUMBER)
> diff --git a/testsuite/awk.tests b/testsuite/awk.tests
> index 93e25d8c1..79e80176c 100755
> --- a/testsuite/awk.tests
> +++ b/testsuite/awk.tests
> @@ -479,4 +479,10 @@ testing 'awk backslash+newline eaten with no trace' \
>   "Hello world\n" \
>   '' ''
>  
> +testing 'awk use-after-free (CVE-2022-30065)' \
> + "awk '\$3i\$3in\$9=\$r||\$9=i6/6-9f'" \
> + "" \
> + "awk: cmd. line:1: Possible syntax error" \
> + 'foo'
> +
>  exit $FAILCOUNT

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] awk: fix use after free (CVE-2022-30065)

[Natanael Copa]

fixes https://bugs.busybox.net/show_bug.cgi?id=14781
---
 editors/awk.c   | 6 --
 testsuite/awk.tests | 6 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/editors/awk.c b/editors/awk.c
index 079d0bde5..be38289e4 100644
--- a/editors/awk.c
+++ b/editors/awk.c
@@ -2921,8 +2921,8 @@ static var *evaluate(node *op, var *res)
 */
if (opinfo & OF_RES2) {
R.v = evaluate(op->r.n, TMPVAR1);
-   //TODO: L.v may be invalid now, set L.v to NULL to 
catch bugs?
-   //L.v = NULL;
+   // L.v may be invalid now, set L.v to NULL to catch bugs
+   L.v = NULL;
if (opinfo & OF_STR2) {
R.s = getvar_s(R.v);
debug_printf_eval("R.s:'%s'\n", R.s);
@@ -3128,6 +3128,8 @@ static var *evaluate(node *op, var *res)
 
case XC( OC_MOVE ):
debug_printf_eval("MOVE\n");
+   if (L.v == NULL)
+   syntax_error(EMSG_POSSIBLE_ERROR);
/* if source is a temporary string, jusk relink it to 
dest */
if (R.v == TMPVAR1
 && !(R.v->type & VF_NUMBER)
diff --git a/testsuite/awk.tests b/testsuite/awk.tests
index 93e25d8c1..79e80176c 100755
--- a/testsuite/awk.tests
+++ b/testsuite/awk.tests
@@ -479,4 +479,10 @@ testing 'awk backslash+newline eaten with no trace' \
"Hello world\n" \
'' ''
 
+testing 'awk use-after-free (CVE-2022-30065)' \
+   "awk '\$3i\$3in\$9=\$r||\$9=i6/6-9f'" \
+   "" \
+   "awk: cmd. line:1: Possible syntax error" \
+   'foo'
+
 exit $FAILCOUNT
-- 
2.36.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] modinfo: add -k option for kernel version

[Natanael Copa]

It is useful to be able to specify kernel version when generating
initramfs and similar for a kernel version that might not be the running
one.

bloatcheck on x86_64:

function old new   delta
packed_usage   26193   26218 +25
modinfo_main 391 414 +23
.rodata80296   80298  +2
--
(add/remove: 0/0 grow/shrink: 3/0 up/down: 50/0)   Total: 50
bytes
   textdata bss dec hex filename
   834606   141242008  850738   cfb32 busybox_old
   834657   141242008  850789   cfb65 busybox_unstripped

Signed-off-by: Natanael Copa 
---
 modutils/modinfo.c | 30 ++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/modutils/modinfo.c b/modutils/modinfo.c
index 0a86c3296..53bc02880 100644
--- a/modutils/modinfo.c
+++ b/modutils/modinfo.c
@@ -38,17 +38,18 @@ static const char *const shortcuts[] ALIGN_PTR = {
 
 enum {
OPT_0 = (1 << 0), /* \0 as separator */
-   OPT_F = (1 << 1), /* field name */
+   OPT_k = (1 << 1), /* kernel version */
+   OPT_F = (1 << 2), /* field name */
/* first bits are for -nadlp options, the rest are for
 * fields not selectable with "shortcut" options
 */
-   OPT_n = (1 << 2),
-   OPT_TAGS = ((1 << ARRAY_SIZE(shortcuts)) - 1) << 2,
+   OPT_n = (1 << 3),
+   OPT_TAGS = ((1 << ARRAY_SIZE(shortcuts)) - 1) << 3,
 };
 
 static void display(const char *data, const char *pattern)
 {
-   int flag = option_mask32 >> 1; /* shift out -0 bit */
+   int flag = option_mask32 >> 2; /* shift out -0 and -k bits */
if (flag & (flag-1)) {
/* more than one field to show: print "FIELD:" pfx */
int n = printf("%s:", pattern);
@@ -82,7 +83,8 @@ static void modinfo(const char *path, const char *version,
}
}
 
-   for (j = 1; (1<http://lists.busybox.net/mailman/listinfo/busybox

Re: Request for 1.33.2 release

[Natanael Copa]

> On 24 Nov 2021, at 14:31, Denys Vlasenko  wrote:
> 
> On Thu, Nov 11, 2021 at 5:09 PM Natanael Copa  wrote:
>> Hi!
>> 
>> I think it would be nice with a 1.33.2 release, with fixes for all the 
>> recent CVEs.
>> 
>> Those commits should be cherry-picked to 1_33_stable:
>> 
>> 4d4fc5ca5ee4f   (man: fix segfault in "man 1") CVE-2021-42373
> 
> This is not a security bug. man segfaults "safely" by dereferencing
> NULL pointer (as opposed to dereferencing random value), it can't be used to
> see any secret information.

It got a CVE, regardless if it is a real security bug or not.

The problem I am trying to solve is to turn all those security scanners to 
green. They don’t really care if its a real security threat or not. They just 
want green. And its much easier to just patch the bug than to put up a fight 
against a handful of scripted security scanners.


> 
>> 04f052c56ded5   (unlzma: fix a case where we could read before beginning
>>of buffer) CVE-2021-42374
>> 53a7a9cd8c15d   (ash: parser: Fix VSLENGTH parsing with trailing
>>garbage) CVE-2021-42375
>> 1b7a9b68d0e9(hush: fix handling of \^C and "^C") CVE-2021-42376
>> 83a4967e5042(hush: fix handling of "cmd && &") CVE-2021-42377
> 
> These can be included.
> 
>> We can cherry-pick all 61 commits to be sure to cover the
>> CVE-2021-42378 to CVE-2021-42386:
>> 
>>for i in $(git log --format=oneline 1_33_0..1_34_0 -- editors/awk.c \
>> | awk '{print $1}' | tac); do git cherry-pick -x $i|| break; done
> 
> awk changes are too big for a stable release.

Since it is not really possible to confirm that all the listed CVEs are 
actually fixed without including everything up to the 1.34 release, added a 
patch for that for Alpine Linux. This is still better for downstream 
distros/vendors than upgrade to a new major busybox version.

For busybox 1.33:
https://git.alpinelinux.org/aports/tree/main/busybox/awk-fixes.patch?h=3.14-stable
 
<https://git.alpinelinux.org/aports/tree/main/busybox/awk-fixes.patch?h=3.14-stable>

I also backported the patches for busybox 1.32:
https://git.alpinelinux.org/aports/tree/main/busybox/awk-fixes.patch?h=3.13-stable
 
<https://git.alpinelinux.org/aports/tree/main/busybox/awk-fixes.patch?h=3.13-stable>

And for busybox 1.31:
https://git.alpinelinux.org/aports/tree/main/busybox/awk-fixes.patch?h=3.12-stable
 
<https://git.alpinelinux.org/aports/tree/main/busybox/awk-fixes.patch?h=3.12-stable>

Those should help silence the security scanners.

Thanks!___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Request for 1.33.2 release

[Natanael Copa]

Hi!

I think it would be nice with a 1.33.2 release, with fixes for all the recent 
CVEs.

Those commits should be cherry-picked to 1_33_stable:

4d4fc5ca5ee4f   (man: fix segfault in "man 1") CVE-2021-42373
04f052c56ded5   (unlzma: fix a case where we could read before beginning
of buffer) CVE-2021-42374
53a7a9cd8c15d   (ash: parser: Fix VSLENGTH parsing with trailing
garbage) CVE-2021-42375
1b7a9b68d0e9(hush: fix handling of \^C and "^C") CVE-2021-42376
83a4967e5042(hush: fix handling of "cmd && &") CVE-2021-42377


We can cherry-pick all 61 commits to be sure to cover the
CVE-2021-42378 to CVE-2021-42386:

for i in $(git log --format=oneline 1_33_0..1_34_0 -- editors/awk.c \
 | awk '{print $1}' | tac); do git cherry-pick -x $i|| break; done

In other words, run:

git checkout 1_33_stable
git cherry-pick -x 4d4fc5ca5ee4f 04f052c56ded5 53a7a9cd8c15d 1b7a9b68d0e9 
83a4967e5042
for i in $(git log --format=oneline 1_33_0..1_34_0 -- editors/awk.c \
 | awk '{print $1}' | tac); do git cherry-pick -x $i|| break; done
git tag 1_33_2

and push that.


Hopefully there will be no regressions in awk, but at least it will be
less risky than upgrade the entire busybox to 1.34.

Thanks!

-nc

PS. would be great if we also could get similar 1.32.2 and 1.31.1
releases.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Backporting security fixes for 1_31_stable, 1_32_stable and 1_33_stable

[Natanael Copa]

On Thu, 11 Nov 2021 12:33:28 +
xiechengliang  wrote:

> I asked one of the disclosers of these vulnerabilities by email,  he gave me 
> the following results.
> 
>   CVEfix
> CVE-2021-42373commit 4d4fc5ca5ee4f (man: fix segfault in "man 1")
> CVE-2021-42374commit 04f052c56ded (unlzma: fix a case where 
> we could read before beginning of buffer)
> CVE-2021-42375   commit 53a7a9cd8c15 (ash: parser: Fix VSLENGTH parsing 
> with trailing garbage)

This confirms my own investigation of CVE-2021-42375, thanks!

> CVE-2021-42376   commit 1b7a9b68d0e9 (hush: fix handling of \^C and "^C")
> CVE-2021-42377   commit 83a4967e5042 (hush: fix handling of "cmd && &")
> 
> CVE-2021-42378-- CVE-2021-42386,  For the CVE related to the awk,  he
> also doesn't know which patch is for each CVE.

Yeah, the above were the "easy" ones. awk seems significantly more
difficult to find a correct match.
 
> The following is part of the original text of his email:
> " BusyBox maintainers fixed our reported issues across multiple
> commits, especially for the awk utility, so it*s not straight forward
> to find all of the fix commits easily."
> 
> Can anyone point out the repair commit of awk related CVE ?

+1

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Backporting security fixes for 1_31_stable, 1_32_stable and 1_33_stable

[Natanael Copa]

Hi!

There are a number of security issues published that affects busybox
older than 1.34.

https://jfrog.com/blog/unboxing-busybox-14-new-vulnerabilities-uncovered-by-claroty-and-jfrog/

I am interested in backporting the fixes to alpine linux stable
branches, which means that I'd like to backport the fixes for:
- 1_33_stable
- 1_32_stable
- 1_31_stable

So I am trying to find the exact commits that fixes each CVE so I can
document that we fix everything. But I need some help with identifying
the exact commit that fixes each CVE.

I believe others are interested in this as well so I'm sharing my
findings here.

CVE-2021-42373: A NULL pointer dereference in man leads to
denial of service when a section name is supplied but
no page argument is given

man 1.33.0-1.33.1

This issue does not affect alpine linux because we don't have `man`
applet enabled. The upstream fix seems to be: 

commit 4d4fc5ca5ee4f (man: fix segfault in "man 1")



CVE-2021-42374: An out-of-bounds heap read in unlzma leads to
information leak and denial of service when crafted
LZMA-compressed input is decompressed. This can be
triggered by any applet/format that internally supports
LZMA compression.

lzma/unlzma and more1.27.0 * 1.33.1

Upstream fix seems to be commit 04f052c56ded (unlzma: fix a case where
we could read before beginning of buffer)


CVE-2021-42375: An incorrect handling of a special element in
ash leads to denial of service when processing a
crafted shell command, due to the shell mistaking
specific characters for reserved characters. This may
be used for DoS under rare conditions of filtered
command input.

ash 1.33.1

Anyone knows which commit fixes this? It should be one of those, but I
have no clue which:

76ef4391548ded8db511e2f7f8f35a3010be7ec5 ash: regressions in process 
substitution
53d45c934f54b7931cc736eba42903cb1f6d4632 ash: speed up ${v//pattern/repl}
1310d7b1d106d7ab0ec84ce88c12302cca934230 ash: speed up ${v//pattern/repl} if 
!ASH_OPTIMIZE_FOR_SIZE
53a7a9cd8c15d64fcc2278cf8981ba526dfbe0d2 ash: parser: Fix VSLENGTH parsing with 
trailing garbage
ad57e4e4b23926002ce72979729b017520bef1d0 ash: revert accidental change (should 
have been separate)
96436fb36a5fa0ac8e993fb093b4788fb5448afe e2fsprogs/*: remove ioctl calling 
obfuscation
1f60d88cf6f5ad3efcad6e7ef1501ce334046e40 *: more --help tweaks
e2b9215868a3d72691e5bc0f887354254606447b *: --help tweaks
457825f77a7c7286647ee888a1000a6bb12ca8fc shells: do not allow bare "read" in 
non-bash compat configs
a1b0d3856d9a0419cb74bf4c87525265871b5868 ash: add process substitution in 
bash-compatibility mode
33745b1fc8cc6d41f4e708d67800d296668af2ce ash: placate -Werror=format-security
2b7c1aa92c68524559a2067609d09309d5c09adc ash: match bash behavior for 
${empty_var/*/repl}
4e039bab375a273344b6c847daa04f13d8317c04 ash: improve --help
85158b600d161bea3fc9d62df8e4ecf4d642fbf0 ash: code shrink
3c13da3dab539eac948de48640d8862857d0c8d0 libbb: introduce and use 
xgettimeofday(), do not truncate 64-bit time_t in shells

I would guess it is "ash: parser: Fix VSLENGTH parsing with trailing
garbage". Can someone confirm that?



We don't use hush in alpine linux, but someone else might be interested
in backporting those.

CVE-2021-42376: A NULL pointer dereference in hush leads to
denial of service when processing a crafted shell
command, due to missing validation after a \x03
delimiter character. This may be used for DoS under
very rare conditions of filtered command input.

hush1.16-1.31.1

I guess this is commit 1b7a9b68d0e9 (hush: fix handling of \^C and "^C")?


CVE-2021-42377: An attacker-controlled pointer free in hush
leads to denial of service and possible code execution
when processing a crafted shell command, due to the
shell mishandling the &&& string. This may be used for
remote code execution under rare conditions of filtered
command input.

hush1.33.0-1.33.1

I guess this is commit 83a4967e5042 (hush: fix handling of "cmd && &")?




The list of awk fixes is a bit more complicated. I have no clue which
of the following commits fixes which CVE:

$ git log --format=oneline 1_33_0..1_34_0 -- editors/awk.c 
dabbeeb79356eef78528acd55e1f143ae80372f7 awk: whitespace and debugging tweaks
d3480dd58211d9d8c06ec7ef00089262603003ff awk: disallow break/continue outside 
of loops
d62627487a44d9175b05d49846aeef83fed97019 awk: tighten parsing - disallow extra 
semicolons
ab755e3717cefc06fd28ce8db56f0402412afaa3 awk: in parsing, remove superfluous 
NEWLINE check; optimize builtin arg evaluation
8d269ef85984f6476e7fdbec2c5a70f3b5c48a72 awk: fix printf 

[PATCH] lineedit: fix tab completion with equal sign

[Natanael Copa]

Fix tab completion for the path when equal sign (=) is used. For
example: dd if=/dev/ze

Signed-off-by: Natanael Copa 
---

I have used busybox ash as the primary shell for over a decade. This
tiny feature will make a pretty big impact.

 libbb/lineedit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libbb/lineedit.c b/libbb/lineedit.c
index b0adcf140..2cae4711a 100644
--- a/libbb/lineedit.c
+++ b/libbb/lineedit.c
@@ -1071,7 +1071,7 @@ static NOINLINE int build_match_prefix(char *match_buf)
continue;
for (--i; i >= 0; i--) {
int cur = int_buf[i];
-   if (cur == ' ' || cur == '<' || cur == '>' || cur == '|' || cur 
== '&') {
+   if (cur == ' ' || cur == '<' || cur == '>' || cur == '|' || cur 
== '&' || cur == '=') {
remove_chunk(int_buf, 0, i + 1);
break;
}
-- 
2.31.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2] echo: do not assume that free() leaves errno unmodified

[Natanael Copa]

On Fri, 22 Jan 2021 08:35:55 +0100
Natanael Copa  wrote:

> musl libc's mallocng free() may modify errno if kernel does not support
> MADV_FREE which causes echo to echo with error when it shouldn't.
> 
> Future versions of POSIX[1] will require that free() leaves errno
> unmodified but til then, do not rely free() implementation.
> 
> Should fix downstream issues:
> https://github.com/alpinelinux/docker-alpine/issues/134
> https://gitlab.alpinelinux.org/alpine/aports/-/issues/12311
> 
> Bloatcheck on x86_64:
> 
> function old new   delta
> echo_main414 406  -8
> --
> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-8)   Total: -8
> bytes
>text  data bss dec hex filename
> 881114  151962000  898310   db506 busybox_old
>  881106 151962000  898302   db4fe busybox_unstripped
> ---
> 
> v2: do the free after bb_simple_perror_msg so it prints the correct
> error message.
>  
>  coreutils/echo.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/coreutils/echo.c b/coreutils/echo.c
> index b3828894c..002832ead 100644
> --- a/coreutils/echo.c
> +++ b/coreutils/echo.c
> @@ -97,6 +97,7 @@ int echo_main(int argc UNUSED_PARAM, char **argv)
>  #else
>   char nflag = 1;
>   char eflag = 0;
> + int err;
>  
>   while ((arg = *++argv) != NULL) {
>   char n, e;
> @@ -184,14 +185,12 @@ int echo_main(int argc UNUSED_PARAM, char **argv)
>  
>   do_write:
>   /* Careful to error out on partial writes too (think ENOSPC!) */
> - errno = 0;
> - /*r =*/ full_write(STDOUT_FILENO, buffer, out - buffer);
> - free(buffer);
> - if (/*WRONG:r < 0*/ errno) {
> + err = full_write(STDOUT_FILENO, buffer, out - buffer) != out - buffer;
> + if (err) {
>   bb_simple_perror_msg(bb_msg_write_error);
> - return 1;
>   }
> - return 0;
> + free(buffer);
> + return err;
>  }
>  
>  /*

Ping. Any change to get this merged?

It fixes a real bug and saves a few bytes.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH v2] echo: do not assume that free() leaves errno unmodified

[Natanael Copa]

musl libc's mallocng free() may modify errno if kernel does not support
MADV_FREE which causes echo to echo with error when it shouldn't.

Future versions of POSIX[1] will require that free() leaves errno
unmodified but til then, do not rely free() implementation.

Should fix downstream issues:
https://github.com/alpinelinux/docker-alpine/issues/134
https://gitlab.alpinelinux.org/alpine/aports/-/issues/12311

Bloatcheck on x86_64:

function old new   delta
echo_main414 406  -8
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-8)   Total: -8
bytes
   textdata bss dec hex filename
881114151962000  898310   db506 busybox_old
 881106   151962000  898302   db4fe busybox_unstripped
---

v2: do the free after bb_simple_perror_msg so it prints the correct
error message.
 
 coreutils/echo.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/coreutils/echo.c b/coreutils/echo.c
index b3828894c..002832ead 100644
--- a/coreutils/echo.c
+++ b/coreutils/echo.c
@@ -97,6 +97,7 @@ int echo_main(int argc UNUSED_PARAM, char **argv)
 #else
char nflag = 1;
char eflag = 0;
+   int err;
 
while ((arg = *++argv) != NULL) {
char n, e;
@@ -184,14 +185,12 @@ int echo_main(int argc UNUSED_PARAM, char **argv)
 
  do_write:
/* Careful to error out on partial writes too (think ENOSPC!) */
-   errno = 0;
-   /*r =*/ full_write(STDOUT_FILENO, buffer, out - buffer);
-   free(buffer);
-   if (/*WRONG:r < 0*/ errno) {
+   err = full_write(STDOUT_FILENO, buffer, out - buffer) != out - buffer;
+   if (err) {
bb_simple_perror_msg(bb_msg_write_error);
-   return 1;
}
-   return 0;
+   free(buffer);
+   return err;
 }
 
 /*
-- 
2.30.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] echo: do not assume that free() leaves errno unmodified

[Natanael Copa]

musl libc's mallocng free() may modify errno if kernel does not support
MADV_FREE which causes echo to echo with error when it shouldn't.

Future versions of POSIX[1] will require that free() leaves errno
unmodified but til then, do not rely free() implementation.

Should fix downstream issues:
https://github.com/alpinelinux/docker-alpine/issues/134
https://gitlab.alpinelinux.org/alpine/aports/-/issues/12311

Bloatcheck on x86_64:

function old new   delta
echo_main414 406  -8
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-8)   Total: -8
bytes
   textdata bss dec hex filename
881114151962000  898310   db506 busybox_old
 881106   151962000  898302   db4fe busybox_unstripped
---
 coreutils/echo.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/coreutils/echo.c b/coreutils/echo.c
index b3828894c..9e8939a00 100644
--- a/coreutils/echo.c
+++ b/coreutils/echo.c
@@ -97,6 +97,7 @@ int echo_main(int argc UNUSED_PARAM, char **argv)
 #else
char nflag = 1;
char eflag = 0;
+   int err;
 
while ((arg = *++argv) != NULL) {
char n, e;
@@ -184,14 +185,12 @@ int echo_main(int argc UNUSED_PARAM, char **argv)
 
  do_write:
/* Careful to error out on partial writes too (think ENOSPC!) */
-   errno = 0;
-   /*r =*/ full_write(STDOUT_FILENO, buffer, out - buffer);
+   err = full_write(STDOUT_FILENO, buffer, out - buffer) != out - buffer;
free(buffer);
-   if (/*WRONG:r < 0*/ errno) {
+   if (err) {
bb_simple_perror_msg(bb_msg_write_error);
-   return 1;
}
-   return 0;
+   return err;
 }
 
 /*
-- 
2.30.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

deadlock in ash in busybox 1.32

[Natanael Copa]

Hi!

The testsuite for stunnel-5.56 deadlocks after busybox upgrade from
1.32.1->1.32.0.

I have bisected it to this commmit:

commit 47eb979404735b9528538968cb5eaac7355a0c5a (HEAD, refs/bisect/bad)
Author: Denys Vlasenko 
Date:   Tue Feb 18 15:37:43 2020 +0100

ash: jobs: Only clear gotsigchld when waiting for everything

I have also confirmed that reverting that commit solves the deadlock.

To reproduce:

* fetch https://www.stunnel.org/downloads/archive/5.x/stunnel-5.56.tar.gz
* build it from source.
* run make check with busybox ash (/bin/sh can be edited in tests/make_test)

I am not sure what the proper fix is.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Non-responsive maintainer?

[Natanael Copa]

On Fri, 5 Jun 2020 12:10:54 +0200
Bernhard Reutner-Fischer  wrote:
 
> PS: Furthermore don't forget that Denys is doing this stuff for more
> than 10 years now and in these years has "donated" quite alot of his
> spare time to the project. One usually does not hear "thanks alot for
> your continued dedication" when being maintainer.

It is 14 years in September this year. (https://busybox.net/oldnews.html)

Denys,

Thank you for you for your continued dedication! It is highly
appreciated.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH v2 2/2] iproute: codeshrink initalization of netlink request

[Natanael Copa]

reduce size by de-duplicate initialization of netlink reqest structure
initialization.

function old new   delta
init_rtnlmsg_rtmsg_req -  34 +34
init_nlmsg_ifinfomsg_req   -  34 +34
set_master   191 171 -20
iproute_modify  12411218 -23
iproute_get  880 857 -23
do_add_or_delete13441319 -25
iprule_modify955 928 -27
--

v2: unmodified, but re-sent for convienience.

(add/remove: 2/0 grow/shrink: 0/5 up/down: 68/-118)   Total: -50
bytes
   textdata bss dec hex filename
 876517   156362080  894233   da519 busybox_old
 876467   156362080  894183   da4e7 busybox_unstripped
---
 networking/libiproute/ip_common.h |  7 ++
 networking/libiproute/iplink.c| 37 ++-
 networking/libiproute/iproute.c   | 35 -
 networking/libiproute/iprule.c| 12 ++
 4 files changed, 40 insertions(+), 51 deletions(-)

diff --git a/networking/libiproute/ip_common.h 
b/networking/libiproute/ip_common.h
index 15f9bb4df..5230a5b20 100644
--- a/networking/libiproute/ip_common.h
+++ b/networking/libiproute/ip_common.h
@@ -15,6 +15,13 @@
 
 PUSH_AND_SET_FUNCTION_VISIBILITY_TO_HIDDEN
 
+struct rtnlmsg_rtmsg_req {
+   struct nlmsghdr n;
+   struct rtmsgr;
+   charbuf[1024];
+};
+
+void FAST_FUNC init_rtnlmsg_rtmsg_req(struct rtnlmsg_rtmsg_req *req);
 char FAST_FUNC **ip_parse_common_args(char **argv);
 //int FAST_FUNC print_neigh(struct sockaddr_nl *who, struct nlmsghdr *n, void 
*arg);
 int FAST_FUNC ipaddr_list_or_flush(char **argv, int flush);
diff --git a/networking/libiproute/iplink.c b/networking/libiproute/iplink.c
index 1a1064bdc..32a0613e0 100644
--- a/networking/libiproute/iplink.c
+++ b/networking/libiproute/iplink.c
@@ -43,6 +43,12 @@ struct ifla_vlan_flags {
 };
 #endif
 
+struct nlmsg_ifinfomsg_req {
+   struct nlmsghdr  n;
+   struct ifinfomsg i;
+   char buf[1024];
+};
+
 /* taken from linux/sockios.h */
 #define SIOCSIFNAME  0x8923  /* set interface name */
 
@@ -55,6 +61,13 @@ struct ifla_vlan_flags {
 
 #define str_on_off "on\0""off\0"
 
+static void init_nlmsg_ifinfomsg_req(struct nlmsg_ifinfomsg_req *req) {
+   memset(req, 0, sizeof(*req));
+   req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
+   req->n.nlmsg_flags = NLM_F_REQUEST;
+   req->i.ifi_family = preferred_family;
+}
+
 /* Exits on error */
 static int get_ctl_fd(void)
 {
@@ -131,18 +144,10 @@ static void set_mtu(char *dev, int mtu)
 static void set_master(char *dev, int master)
 {
struct rtnl_handle rth;
-   struct {
-   struct nlmsghdr  n;
-   struct ifinfomsg i;
-   char buf[1024];
-   } req;
-
-   memset(, 0, sizeof(req));
+   struct nlmsg_ifinfomsg_req req;
 
-   req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
-   req.n.nlmsg_flags = NLM_F_REQUEST;
+   init_nlmsg_ifinfomsg_req();
req.n.nlmsg_type = RTM_NEWLINK;
-   req.i.ifi_family = preferred_family;
 
xrtnl_open();
req.i.ifi_index = xll_name_to_index(dev);
@@ -596,11 +601,7 @@ static int do_add_or_delete(char **argv, const unsigned 
rtm)
ARG_address,
};
struct rtnl_handle rth;
-   struct {
-   struct nlmsghdr  n;
-   struct ifinfomsg i;
-   char buf[1024];
-   } req;
+   struct nlmsg_ifinfomsg_req req;
smalluint arg;
char *name_str = NULL;
char *link_str = NULL;
@@ -608,12 +609,8 @@ static int do_add_or_delete(char **argv, const unsigned 
rtm)
char *dev_str = NULL;
char *address_str = NULL;
 
-   memset(, 0, sizeof(req));
-
-   req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
-   req.n.nlmsg_flags = NLM_F_REQUEST;
+   init_nlmsg_ifinfomsg_req();
req.n.nlmsg_type = rtm;
-   req.i.ifi_family = preferred_family;
if (rtm == RTM_NEWLINK)
req.n.nlmsg_flags |= NLM_F_CREATE|NLM_F_EXCL;
 
diff --git a/networking/libiproute/iproute.c b/networking/libiproute/iproute.c
index 9a58fdbb5..cf562fd5f 100644
--- a/networking/libiproute/iproute.c
+++ b/networking/libiproute/iproute.c
@@ -348,6 +348,15 @@ static int str_is_lock(const char *str)
return strcmp(str, "lock") == 0;
 }
 
+void init_rtnlmsg_rtmsg_req(struct rtnlmsg_rtmsg_req *req)
+{
+   memset(req, 0, sizeof(*req));
+   req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg));
+   if (NLM_F_REQUEST)
+   req->n.nlmsg_flags = 

[PATCH v2 1/2] iproute: codeshrink netlink message length check

[Natanael Copa]

reduce size by de-duplicate netlink message length check

function old new   delta
check_nlmsg_len-  22 +22
print_route 19011893  -8
print_neigh 10821073  -9
iproute_get  889 880  -9
.rodata   139741  139723 -18
--

v2:
 - use void instead of int for check_nlmsg_len()

(add/remove: 1/0 grow/shrink: 0/4 up/down: 22/-44)Total: -22
bytes
   textdata bss dec hex filename
 876539   156362080  894255   da52f busybox_old
 876517   156362080  894233   da519 busybox_unstripped
---
 networking/libiproute/ip_common.h |  1 +
 networking/libiproute/ipneigh.c   |  4 +---
 networking/libiproute/iproute.c   | 14 +-
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/networking/libiproute/ip_common.h 
b/networking/libiproute/ip_common.h
index 40171bed9..15f9bb4df 100644
--- a/networking/libiproute/ip_common.h
+++ b/networking/libiproute/ip_common.h
@@ -30,6 +30,7 @@ int FAST_FUNC do_iplink(char **argv);
 //int FAST_FUNC do_ipmonitor(char **argv);
 //int FAST_FUNC do_multiaddr(char **argv);
 //int FAST_FUNC do_multiroute(char **argv);
+void FAST_FUNC check_nlmsg_len(int len);
 
 POP_SAVED_FUNCTION_VISIBILITY
 
diff --git a/networking/libiproute/ipneigh.c b/networking/libiproute/ipneigh.c
index b9b4f4b31..8771a9ad9 100644
--- a/networking/libiproute/ipneigh.c
+++ b/networking/libiproute/ipneigh.c
@@ -102,9 +102,7 @@ static int FAST_FUNC print_neigh(const struct sockaddr_nl 
*who UNUSED_PARAM,
 n->nlmsg_flags);
}
len -= NLMSG_LENGTH(sizeof(*r));
-   if (len < 0) {
-   bb_error_msg_and_die("BUG: wrong nlmsg len %d", len);
-   }
+   check_nlmsg_len(len);
 
if (G_filter.flushb && n->nlmsg_type != RTM_NEWNEIGH)
return 0;
diff --git a/networking/libiproute/iproute.c b/networking/libiproute/iproute.c
index 5a972f8b2..9a58fdbb5 100644
--- a/networking/libiproute/iproute.c
+++ b/networking/libiproute/iproute.c
@@ -64,6 +64,13 @@ static int flush_update(void)
return 0;
 }
 
+void check_nlmsg_len(int len)
+{
+   if (len < 0) {
+   bb_error_msg_and_die("wrong nlmsg len %d", len);
+   }
+}
+
 static int FAST_FUNC print_route(const struct sockaddr_nl *who UNUSED_PARAM,
struct nlmsghdr *n, void *arg UNUSED_PARAM)
 {
@@ -83,8 +90,7 @@ static int FAST_FUNC print_route(const struct sockaddr_nl 
*who UNUSED_PARAM,
if (G_filter.flushb && n->nlmsg_type != RTM_NEWROUTE)
return 0;
len -= NLMSG_LENGTH(sizeof(*r));
-   if (len < 0)
-   bb_error_msg_and_die("wrong nlmsg len %d", len);
+   check_nlmsg_len(len);
 
//memset(tb, 0, sizeof(tb)); - parse_rtattr does this
parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
@@ -1080,9 +1086,7 @@ static int iproute_get(char **argv)
bb_simple_error_msg_and_die("not a route?");
}
len -= NLMSG_LENGTH(sizeof(*r));
-   if (len < 0) {
-   bb_error_msg_and_die("wrong len %d", len);
-   }
+   check_nlmsg_len(len);
 
//memset(tb, 0, sizeof(tb)); - parse_rtattr does this
parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
-- 
2.25.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] iproute: codeshrink netlink message length check

[Natanael Copa]

On Wed, 13 Nov 2019 18:55:09 +0100
Bernhard Reutner-Fischer  wrote:

> On 12 November 2019 12:04:24 CET, Natanael Copa  wrote:
> >reduce size by de-duplicate netlink message length check
> >
> >function old new  
> >delta
> >check_nlmsg_len-  22
> >+22
> >print_route 19011893 
> >-8
> >print_neigh 10821073 
> >-9
> >iproute_get  889 880 
> >-9
> >.rodata   139741  139723
> >-18
> >--
> >(add/remove: 1/0 grow/shrink: 0/4 up/down: 22/-44)Total:
> >-22
> >bytes
> >   text data bss dec hex filename
> > 876539156362080  894255   da52f busybox_old
> > 876517156362080  894233   da519 busybox_unstripped
> >---
> > networking/libiproute/ip_common.h |  1 +
> > networking/libiproute/ipneigh.c   |  4 +---
> > networking/libiproute/iproute.c   | 14 +-
> > 3 files changed, 11 insertions(+), 8 deletions(-)
> >
> >diff --git a/networking/libiproute/ip_common.h
> >b/networking/libiproute/ip_common.h
> >index 40171bed9..9f7f927c7 100644
> >--- a/networking/libiproute/ip_common.h
> >+++ b/networking/libiproute/ip_common.h
> >@@ -30,6 +30,7 @@ int FAST_FUNC do_iplink(char **argv);
> > //int FAST_FUNC do_ipmonitor(char **argv);
> > //int FAST_FUNC do_multiaddr(char **argv);
> > //int FAST_FUNC do_multiroute(char **argv);
> >+int FAST_FUNC check_nlmsg_len(int len);  
> 
> void ?

Yes, of course, it should be void.

Thanks!

-nc

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] nslookup: fix exitcode with search domains, closes 12541

[Natanael Copa]

Return success if there are any valid answer rather than exit with error.

bloatcheck on x86_64:

function old new   delta
nslookup_main917 924  +7
send_queries21152121  +6
.rodata   136606  136609  +3
--
(add/remove: 0/0 grow/shrink: 3/0 up/down: 16/0)   Total: 16
bytes
   textdata bss dec hex filename
 839394   146602080  856134   d1046 busybox_old
 839410   146602080  856150   d1056 busybox_unstripped

Signed-off-by: Natanael Copa 
---
 networking/nslookup.c | 34 ++
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/networking/nslookup.c b/networking/nslookup.c
index c43e60558..d14412386 100644
--- a/networking/nslookup.c
+++ b/networking/nslookup.c
@@ -265,7 +265,7 @@ struct query {
const char *name;
unsigned qlen;
 // unsigned latency;
-// uint8_t rcode;
+   uint8_t rcode;
unsigned char query[512];
 // unsigned char reply[512];
 };
@@ -321,7 +321,7 @@ struct globals {
struct query *query;
char *search;
smalluint have_search_directive;
-   smalluint exitcode;
+   smallint answer;
 } FIX_ALIASING;
 #define G (*(struct globals*)bb_common_bufsiz1)
 #define INIT_G() do { \
@@ -625,28 +625,31 @@ static int send_queries(struct ns *ns)
 
/* Process reply */
G.query[qn].qlen = 0; /* flag: "reply received" */
+   G.query[qn].rcode = rcode;
tcur = monotonic_ms();
 #if 1
if (option_mask32 & OPT_debug) {
printf("Query #%d completed in %ums:\n", qn, tcur - 
tstart);
}
if (rcode != 0) {
-   printf("** server can't find %s: %s\n",
-   G.query[qn].name, rcodes[rcode]);
-   G.exitcode = EXIT_FAILURE;
+   printf("** server can't find %s: %s\n\n",
+   G.query[qn].name, rcodes[rcode]);
} else {
switch (parse_reply(reply, recvlen)) {
case -1:
-   printf("*** Can't find %s: Parse error\n", 
G.query[qn].name);
-   G.exitcode = EXIT_FAILURE;
+   printf("*** Can't find %s: Parse error\n\n", 
G.query[qn].name);
+   G.query[qn].rcode = 0x10;
break;
 
case 0:
-   printf("*** Can't find %s: No answer\n", 
G.query[qn].name);
+   G.query[qn].rcode = 0x20;
+   break;
+   default:
+   bb_putchar('\n');
+   G.answer++;
break;
}
}
-   bb_putchar('\n');
n_replies++;
if (n_replies >= G.query_count)
goto ret;
@@ -1004,21 +1007,20 @@ int nslookup_main(int argc UNUSED_PARAM, char **argv)
}
 
err = 0;
-   for (rc = 0; rc < G.query_count; rc++) {
-   if (G.query[rc].qlen) {
-   printf("*** Can't find %s: No answer\n", 
G.query[rc].name);
-   err = 1;
+   for (rc = 0; !G.answer && rc < G.query_count; rc++) {
+   if (G.query[rc].qlen || G.query[rc].rcode == 0x20) {
+   printf("*** Can't find %s: No answer\n\n", 
G.query[rc].name);
+   } else if (G.query[rc].rcode != 0) {
+   err = EXIT_FAILURE;
}
}
-   if (err) /* should this affect exicode too? */
-   bb_putchar('\n');
 
if (ENABLE_FEATURE_CLEAN_UP) {
free(G.server);
free(G.query);
}
 
-   return G.exitcode;
+   return err;
 }
 
 #endif
-- 
2.25.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Sime makelike wrapper scripts support

[Natanael Copa]

On Mon, 13 Jan 2020 16:03:21 +0100
Boris Kotov  wrote:

> > How about using shell functions instead:
> >
> > ```
> > #!/bin/sh -e
> >
> > hello () {
> > echo hello
> > }
> >
> > build() {
> > docker build -t image .
> > }
> >
> > push() {
> > docker push -t image
> > }
> >
> > "$@"
> > ```
> >
> > If you call that minimake.sh, then you call it as:
> >
> > ```
> > ./minimake.sh hello
> > ./minimake.sh build
> > ./minimake.sh push
> > ```  
> 
> 
> Hallo Natanael Copa, thank you for your attention to this topic!
> 
> Sure, you can write shell scripts, functions, case-statements, but with 
> `mim` you eliminate unnecessary noise.

It solves the problem you called "blackbox approach". You no longer
need to open multiple small files.

> For example in the script above there are 5 Lines of 11 which do not 
> provide any value, but noise.

The noise provides value, because it makes it crystal clear what is
happening "under the hood".

> (like "$@" from the bottom of your script, 
> https://stackoverflow.com/questions/9994295/what-does-mean-in-a-shell-script 
> <https://stackoverflow.com/questions/9994295/what-does-mean-in-a-shell-script>
>  
> (174k views))

Its only POSIX shell syntax. You'd still need to learn that, regardless
if you use make or not. With make you need to learn both make syntax
*and* shell syntax, because make will execute a shell for each line.

> So, the USP here is that mim provides a very clear and intuitive 
> interface for non-experienced shell users

Its not that intuitive, even for tiny examples

--- Mimfile ---
hello:
false; echo "will this echo or not?"

hello2:
false
echo "will this echo or not?"

-

and what about variables and variable expansion?


--- Mimfile -

last="1.0"

hello:
echo "Last version is $(last)"

--

What should happen? GNU make and shell handles that different.

In both those cases both experienced and non-experienced users would
need to verify the documentation.


> And for complex stuff, use mim as a wrapper and write shell scripts 
> underneath.

I though the problem mim was supposed to solve was to avoid write
wrapper scripts?

> 
> ```
> 
> deploy:
> 
>    scripts/deploy.sh
> 
> cleanup:
> 
>    rm -rf ./dist
> 
> ```
> 
> 
> I really would love to have it in busybox/alpine and in their 
> descendants, if you have common tasks to use in your containers, just 
> COPY a Mimfile in there.
> 
> ```
> 
> # postgres-container/WORKDIR/Mimfile
> 
> backup:
> 
>    # make a backup, post to s3
> 
> restore:
> 
>    # get from s3 & restore
> 
> ```

So basically, you want a new shell language only to avoid writing '()
{}' when declaring functions.

> 
> 
> ```
> 
> $ docker exec container mim restore
> 
> ```
> 
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Sime makelike wrapper scripts support

[Natanael Copa]

On Mon, 13 Jan 2020 10:30:59 +
Ron Yorston  wrote:

> Boris wrote:
> >Sure, to avoid any confusion I would't use the name "Makefile". 
> >"Minimake" sounds clear to me, and the cli `mim`.  
> 
> I like the command name 'mim' (which Merriam-Webster tells me is an
> adjective meaning 'affectedly shy or modest').  By extension I think the
> file should be called 'Mimfile'.  This avoids any misleading reference to
> make.  As Eli correctly points out (and I should have) the script misses
> the main distinguishing feature of make, that it handles dependencies.
> 
> I've tidied up the script a bit.  Its usage is:
> 
>mim [-f FILE] [SHELL_OPTIONS] [TARGET] ...
> 


Other implementation that converts the "targets" to shell functions:

--- mim 
#!/bin/sh
  
sed -E -e '/^[a-z]+:$/,/^$/{
/^[a-z]/s/:$/() \{/
s/^$/\}/
}' -e '$a for i; do $i; done' Mimfile | sh -e -s "$@"

-

This will run all specified targets instead of passing options.

(I'm still not convinced it is a good idea to add this to busybox)

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Sime makelike wrapper scripts support

[Natanael Copa]

On Sun, 12 Jan 2020 00:10:09 +0100
Boris Kotov  wrote:

> Hello BusyBox community,
> 
> 
> my name is Boris, I would like to make a small feature request regarding 
> the `ash` shell.
> 
> It would be really handy to have a simple Makefile-like wrapper scripts 
> syntax just to execute some common commands on it:
> 
> 
> ```
> 
> hello:
> 
>     echo hello
> 
> build:
> 
>     docker build -t image .
> 
> push:
> 
>     docker push -t image
> 
> ```
> 
> 
> Right now, if you want to have that on alpine you would have to include 
> make, which is about 230kb!!
> 
> 
> Most people are doing this blackbox approach:
> 
> ```
> 
> scripts/
> 
>    hello.sh
> 
>    build.sh
> 
>    push.sh
> 
> ```

How about using shell functions instead:

```
#!/bin/sh -e

hello () {
echo hello
}

build() {
docker build -t image .
}

push() {
docker push -t image
}

"$@"
```

If you call that minimake.sh, then you call it as:

```
./minimake.sh hello
./minimake.sh build
./minimake.sh push
```


> 
> 
> Blackbox, because its not clear what's in there, you have open each 
> file, then your whole terminal window or IDE fills an empty file with 
> one/two valuable lines.
> 
> However Makefile syntax is very nice, could we just include something 
> like `minimake` or called it (do) which only supports naming tasks?
> 
> I think this is such a basic feature, that every language has build its 
> own (mix, npm, rake, etc..)
> 
> Why not include it directly into the most minimal basic shell itself to 
> make our scripts simpler, and the world better.


> 
> 
> Thank you very much
> 
> Boris
> 
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 1/2] iproute: codeshrink netlink message length check

[Natanael Copa]

reduce size by de-duplicate netlink message length check

function old new   delta
check_nlmsg_len-  22 +22
print_route 19011893  -8
print_neigh 10821073  -9
iproute_get  889 880  -9
.rodata   139741  139723 -18
--
(add/remove: 1/0 grow/shrink: 0/4 up/down: 22/-44)Total: -22
bytes
   textdata bss dec hex filename
 876539   156362080  894255   da52f busybox_old
 876517   156362080  894233   da519 busybox_unstripped
---
 networking/libiproute/ip_common.h |  1 +
 networking/libiproute/ipneigh.c   |  4 +---
 networking/libiproute/iproute.c   | 14 +-
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/networking/libiproute/ip_common.h 
b/networking/libiproute/ip_common.h
index 40171bed9..9f7f927c7 100644
--- a/networking/libiproute/ip_common.h
+++ b/networking/libiproute/ip_common.h
@@ -30,6 +30,7 @@ int FAST_FUNC do_iplink(char **argv);
 //int FAST_FUNC do_ipmonitor(char **argv);
 //int FAST_FUNC do_multiaddr(char **argv);
 //int FAST_FUNC do_multiroute(char **argv);
+int FAST_FUNC check_nlmsg_len(int len);
 
 POP_SAVED_FUNCTION_VISIBILITY
 
diff --git a/networking/libiproute/ipneigh.c b/networking/libiproute/ipneigh.c
index b9b4f4b31..8771a9ad9 100644
--- a/networking/libiproute/ipneigh.c
+++ b/networking/libiproute/ipneigh.c
@@ -102,9 +102,7 @@ static int FAST_FUNC print_neigh(const struct sockaddr_nl 
*who UNUSED_PARAM,
 n->nlmsg_flags);
}
len -= NLMSG_LENGTH(sizeof(*r));
-   if (len < 0) {
-   bb_error_msg_and_die("BUG: wrong nlmsg len %d", len);
-   }
+   check_nlmsg_len(len);
 
if (G_filter.flushb && n->nlmsg_type != RTM_NEWNEIGH)
return 0;
diff --git a/networking/libiproute/iproute.c b/networking/libiproute/iproute.c
index 5a972f8b2..a858d57dc 100644
--- a/networking/libiproute/iproute.c
+++ b/networking/libiproute/iproute.c
@@ -64,6 +64,13 @@ static int flush_update(void)
return 0;
 }
 
+int check_nlmsg_len(int len)
+{
+   if (len < 0) {
+   bb_error_msg_and_die("wrong nlmsg len %d", len);
+   }
+}
+
 static int FAST_FUNC print_route(const struct sockaddr_nl *who UNUSED_PARAM,
struct nlmsghdr *n, void *arg UNUSED_PARAM)
 {
@@ -83,8 +90,7 @@ static int FAST_FUNC print_route(const struct sockaddr_nl 
*who UNUSED_PARAM,
if (G_filter.flushb && n->nlmsg_type != RTM_NEWROUTE)
return 0;
len -= NLMSG_LENGTH(sizeof(*r));
-   if (len < 0)
-   bb_error_msg_and_die("wrong nlmsg len %d", len);
+   check_nlmsg_len(len);
 
//memset(tb, 0, sizeof(tb)); - parse_rtattr does this
parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
@@ -1080,9 +1086,7 @@ static int iproute_get(char **argv)
bb_simple_error_msg_and_die("not a route?");
}
len -= NLMSG_LENGTH(sizeof(*r));
-   if (len < 0) {
-   bb_error_msg_and_die("wrong len %d", len);
-   }
+   check_nlmsg_len(len);
 
//memset(tb, 0, sizeof(tb)); - parse_rtattr does this
parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
-- 
2.23.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 2/2] iproute: codeshrink initalization of netlink request

[Natanael Copa]

reduce size by de-duplicate initialization of netlink reqest structure
initialization.

function old new   delta
init_rtnlmsg_rtmsg_req -  34 +34
init_nlmsg_ifinfomsg_req   -  34 +34
set_master   191 171 -20
iproute_modify  12411218 -23
iproute_get  880 857 -23
do_add_or_delete13441319 -25
iprule_modify955 928 -27
--
(add/remove: 2/0 grow/shrink: 0/5 up/down: 68/-118)   Total: -50
bytes
   textdata bss dec hex filename
 876517   156362080  894233   da519 busybox_old
 876467   156362080  894183   da4e7 busybox_unstripped
---
 networking/libiproute/ip_common.h |  7 ++
 networking/libiproute/iplink.c| 37 ++-
 networking/libiproute/iproute.c   | 35 -
 networking/libiproute/iprule.c| 12 ++
 4 files changed, 40 insertions(+), 51 deletions(-)

diff --git a/networking/libiproute/ip_common.h 
b/networking/libiproute/ip_common.h
index 9f7f927c7..1db51eff6 100644
--- a/networking/libiproute/ip_common.h
+++ b/networking/libiproute/ip_common.h
@@ -15,6 +15,13 @@
 
 PUSH_AND_SET_FUNCTION_VISIBILITY_TO_HIDDEN
 
+struct rtnlmsg_rtmsg_req {
+   struct nlmsghdr n;
+   struct rtmsgr;
+   charbuf[1024];
+};
+
+void FAST_FUNC init_rtnlmsg_rtmsg_req(struct rtnlmsg_rtmsg_req *req);
 char FAST_FUNC **ip_parse_common_args(char **argv);
 //int FAST_FUNC print_neigh(struct sockaddr_nl *who, struct nlmsghdr *n, void 
*arg);
 int FAST_FUNC ipaddr_list_or_flush(char **argv, int flush);
diff --git a/networking/libiproute/iplink.c b/networking/libiproute/iplink.c
index 1a1064bdc..32a0613e0 100644
--- a/networking/libiproute/iplink.c
+++ b/networking/libiproute/iplink.c
@@ -43,6 +43,12 @@ struct ifla_vlan_flags {
 };
 #endif
 
+struct nlmsg_ifinfomsg_req {
+   struct nlmsghdr  n;
+   struct ifinfomsg i;
+   char buf[1024];
+};
+
 /* taken from linux/sockios.h */
 #define SIOCSIFNAME  0x8923  /* set interface name */
 
@@ -55,6 +61,13 @@ struct ifla_vlan_flags {
 
 #define str_on_off "on\0""off\0"
 
+static void init_nlmsg_ifinfomsg_req(struct nlmsg_ifinfomsg_req *req) {
+   memset(req, 0, sizeof(*req));
+   req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
+   req->n.nlmsg_flags = NLM_F_REQUEST;
+   req->i.ifi_family = preferred_family;
+}
+
 /* Exits on error */
 static int get_ctl_fd(void)
 {
@@ -131,18 +144,10 @@ static void set_mtu(char *dev, int mtu)
 static void set_master(char *dev, int master)
 {
struct rtnl_handle rth;
-   struct {
-   struct nlmsghdr  n;
-   struct ifinfomsg i;
-   char buf[1024];
-   } req;
-
-   memset(, 0, sizeof(req));
+   struct nlmsg_ifinfomsg_req req;
 
-   req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
-   req.n.nlmsg_flags = NLM_F_REQUEST;
+   init_nlmsg_ifinfomsg_req();
req.n.nlmsg_type = RTM_NEWLINK;
-   req.i.ifi_family = preferred_family;
 
xrtnl_open();
req.i.ifi_index = xll_name_to_index(dev);
@@ -596,11 +601,7 @@ static int do_add_or_delete(char **argv, const unsigned 
rtm)
ARG_address,
};
struct rtnl_handle rth;
-   struct {
-   struct nlmsghdr  n;
-   struct ifinfomsg i;
-   char buf[1024];
-   } req;
+   struct nlmsg_ifinfomsg_req req;
smalluint arg;
char *name_str = NULL;
char *link_str = NULL;
@@ -608,12 +609,8 @@ static int do_add_or_delete(char **argv, const unsigned 
rtm)
char *dev_str = NULL;
char *address_str = NULL;
 
-   memset(, 0, sizeof(req));
-
-   req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
-   req.n.nlmsg_flags = NLM_F_REQUEST;
+   init_nlmsg_ifinfomsg_req();
req.n.nlmsg_type = rtm;
-   req.i.ifi_family = preferred_family;
if (rtm == RTM_NEWLINK)
req.n.nlmsg_flags |= NLM_F_CREATE|NLM_F_EXCL;
 
diff --git a/networking/libiproute/iproute.c b/networking/libiproute/iproute.c
index a858d57dc..e3524c8d1 100644
--- a/networking/libiproute/iproute.c
+++ b/networking/libiproute/iproute.c
@@ -348,6 +348,15 @@ static int str_is_lock(const char *str)
return strcmp(str, "lock") == 0;
 }
 
+void init_rtnlmsg_rtmsg_req(struct rtnlmsg_rtmsg_req *req)
+{
+   memset(req, 0, sizeof(*req));
+   req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg));
+   if (NLM_F_REQUEST)
+   req->n.nlmsg_flags = NLM_F_REQUEST;
+   req->r.rtm_family = 

Re: [PATCH 1/1] Optionally re-introduce bb_info_msg()

[Natanael Copa]

On Fri, 12 Apr 2019 17:01:51 +
James Byrne  wrote:

> Between Busybox 1.24.2 and 1.25.0 the bb_info_msg() function was
> eliminated and calls to it changed to be bb_error_msg(). The downside of
> this is that daemons now log all messages to syslog at the LOG_ERR level
> which makes it hard to filter errors from informational messages.

FWIW, This has been an annoyance in Alpine Linux, so I would very much
to see this patch get applied.

Thanks!

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Natanael Copa]

Denys,

Most common use case for https is to give some sort of guarantee that
you actually get what you think you get or that you get from who you
think you get it from. That is what most people expect when downloading
from https. If you don't care about verifying that, then the common use
case is to use http (without the 's').

Now, I think it is perfectly fine that some people does not care about
checking certificates, but in that case I think its reasonable to
explicitly tell that to wget. This is exactly what the GNU wget does
and this is what Jirutka's patch does. I am confident that this is what
the big majority would want from the tool.

Apparently there are strong opinions in both directions here what the
desired behavior should be, so I think it makes sense to have a config
option for this?

-nc


On Sat, 26 May 2018 19:34:05 +0200
Denys Vlasenko  wrote:

> wget should work for common use cases.
> Such as downloading sources of kernels, gcc and such.
> From build scripts, not only by hand.
> Without having to modify said scripts.
> Your patch breaks that.
> NAK.
> 
> I don't care that security people are upset.
> They are paranoid, it's part of their profession.
> It does not mean everybody else have to be as paranoid.
> 
> If you have a patch which adds actual cert checking
> and thus does not introduce regressions, please post it.
> 
> 
> On Sat, May 26, 2018 at 6:38 PM,   wrote:
> >> //config:   If you still think this is unacceptable, send patches.  
> >
> >
> > That*s exactly what I did.
> > http://lists.busybox.net/pipermail/busybox/2018-May/086444.html
> >
> > Jakub
> >
> >
> > On 2018-05-26 17:54, Denys Vlasenko wrote:  
> >>
> >> On Sat, May 26, 2018 at 5:39 PM,   wrote:  
> >
> > That's a crime against security!  
> 
> 
>  Say what?  
> >>>
> >>>
> >>> That*s a hyperbole. The thing is that when you don*t verify the peer*s
> >>> certificate, then you*re vulnerable to MitM attack with fake certificate
> >>> injection. The whole SSL/TLS is totally useless in that moment. It*s more
> >>> or
> >>> less like putting the door*s key under the carpet right in front of the
> >>> door.
> >>>
> >>> Allowing to bypass/ignore certificate verification is ok-ish in some
> >>> situations, but only when the user do it consciously, using explicit
> >>> option
> >>> such as --no-check-certificate, not silently as the default option.  
> >>
> >>
> >> wget.c:
> >>
> >> //config:   If you still think this is unacceptable, send patches.
> >> //config:
> >> //config:   If you still think this is unacceptable, do not want to
> >> send
> >> //config:   patches, but do want to waste bandwidth explaining how
> >> wrong
> >> //config:   it is, you will be ignored.  

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] testsuite: add test for regression in cat

[Natanael Copa]

Add a couple of tests for a regression introduced with commit 75e90b15
(cat: fix "cat -An" ignoring -n; make numbering go througn all files)

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---

I don't have a fix for the regression yet but I was able to find the
commit which introduces it and made a couple of tests for the testsuite.

The bug causes the testsuite for GNU patch to fail. It took me a while
to figure out that it was a bug in busybox cat and not in GNU patch :)

 testsuite/cat/cat-show-end-of-line | 3 +++
 testsuite/cat/cat-show-non-printing-characters | 3 +++
 2 files changed, 6 insertions(+)
 create mode 100644 testsuite/cat/cat-show-end-of-line
 create mode 100644 testsuite/cat/cat-show-non-printing-characters

diff --git a/testsuite/cat/cat-show-end-of-line 
b/testsuite/cat/cat-show-end-of-line
new file mode 100644
index 0..46945d4d4
--- /dev/null
+++ b/testsuite/cat/cat-show-end-of-line
@@ -0,0 +1,3 @@
+echo foo | busybox cat -e > foo
+echo 'foo$' > bar
+cmp foo bar
diff --git a/testsuite/cat/cat-show-non-printing-characters 
b/testsuite/cat/cat-show-non-printing-characters
new file mode 100644
index 0..541a8bab8
--- /dev/null
+++ b/testsuite/cat/cat-show-non-printing-characters
@@ -0,0 +1,3 @@
+echo foo | busybox cat -v > foo
+echo foo > bar
+cmp foo bar
-- 
2.17.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Switching Fedora machine away from systemd

[Natanael Copa]

On Thu, 19 Apr 2018 17:18:29 +0200
Denys Vlasenko  wrote:

> After yet another incident of wandering among endless
> directories/"targets"/links trying to figure out how to stop
> the laptop from falling asleep on lid close, I've had enough.
> 
> Since no one had time/inspiration to create runit/daemontools
> style servie supervision in Fedora, I'll going to create
> something homegrown.
> 
> This also doubles as a test whether busybox's runit tools
> are up to the task.

Or you could use Alpine Linux which uses busybox and musl libc out of
the box, and with openrc instead of systemd.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: serioius resgression with cpio -p and symlinks in busybox 1.28.2

[Natanael Copa]

On Sat, 31 Mar 2018 16:25:57 +0200
Denys Vlasenko <vda.li...@googlemail.com> wrote:

> I can release a 1.28.3 in a few days if you want. Do you?
> 
> Let's see what else would crop up during weekend.

I think releasing 1.28.3 would be a good idea.

I would also like to have the following commits backported:

  e2afae6303e871a31a061d03359cfcd5dd86c088 sed: prevent overflow of length from 
bb_get_chunk_from_file
  2da9724b56169f00bd7fb6b9a11c9409a7620981 libbb: remove unnecessary variable 
in xmalloc_fgets

Those look like they are fixing a security vulnerability and I think a
CVE should be requested. Note that ifup/ifdown segfaults without the
second commit (libbb: remove unnecessary variable in xmalloc_fgets)

Would also be nice to have grep fixed:

  03fd7e06f854d385070a6fc9714f445727c359cd grep: fix echo "aa" | busybox grep 
-F -w "a" (should not match)

Thanks!

-nc

> 
> On Fri, Mar 30, 2018 at 1:29 PM, Natanael Copa <nc...@alpinelinux.org> wrote:
> > Hi,
> >
> > There is a serious regression in busybox 1.28.2 that breaks booting
> > alpine linux machines.
> >
> > The problem comes from /bin/sh -> /bin/busybox symlink no longer getting 
> > copied with cpio -p.
> >
> > To reproduce:
> >
> > $ mkdir -p 1/bin 2
> > $ ln -s /bin/busybox 1/bin/sh
> > $ (cd 1 && echo "/bin/sh" | cpio -vdmp ../2)
> > bin/sh
> > 1 blocks
> > $ find 2/
> > 2/
> > 2/bin
> >
> > The /bin/sh symlink was silently ignored and this causes the
> > initramfs's #/bin/sh script fail at boot.
> >
> > It was previously possible to work around it by setting
> > EXTRACT_UNSAFE_SYMLINKS=1 but this no longer works.
> >
> > https://git.alpinelinux.org/cgit/aports/commit/main/mkinitfs?id=1b6a167de8ce02d69dc8a8c8f4638aefd27c0ebe
> >
> > Downstream bug report:
> > https://bugs.alpinelinux.org/issues/8751
> >
> > -nc
> > ___
> > busybox mailing list
> > busybox@busybox.net
> > http://lists.busybox.net/mailman/listinfo/busybox  

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] cpio: fix regression with symlinks

[Natanael Copa]

On Fri, 30 Mar 2018 20:16:21 +0200
Denys Vlasenko <vda.li...@googlemail.com> wrote:

> On Fri, Mar 30, 2018 at 2:17 PM, Natanael Copa <nc...@alpinelinux.org> wrote:
> > Fix to make cpio create symlinks.
> >
> > This fixes a regression in cpio introduced with
> > a84db18fc71d09e801df0ebca048d82e90b32c6a (tar,unzip: postpone creation
> > of symlinks with "suspicious" targets).  
> 
> Is it really a regression? When I go back to
> commit 0cf64c8b5d86d603903397bfce87dea5a862caec,
> I don't see any code in cpio which would create these symlinks.
> Looks like it was broken all along?

busybox 1.28.1 worked with EXTRACT_UNSAFE_SYMLINKS=1. It broke with
busybox 1.28.2 upgrade.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] cpio: fix regression with symlinks

[Natanael Copa]

Fix to make cpio create symlinks.

This fixes a regression in cpio introduced with
a84db18fc71d09e801df0ebca048d82e90b32c6a (tar,unzip: postpone creation
of symlinks with "suspicious" targets).

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---

This should got to fixes-1.28.2/

 archival/cpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/archival/cpio.c b/archival/cpio.c
index 1d6cbd1e2..3eb37c603 100644
--- a/archival/cpio.c
+++ b/archival/cpio.c
@@ -507,6 +507,7 @@ int cpio_main(int argc UNUSED_PARAM, char **argv)
archive_handle->cpio__blocks = (off_t)-1;
while (get_header_cpio(archive_handle) == EXIT_SUCCESS)
continue;
+   create_symlinks_from_list(archive_handle->symlink_placeholders);
 
if (archive_handle->cpio__blocks != (off_t)-1
 && !(opt & OPT_QUIET)
-- 
2.16.3

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

serioius resgression with cpio -p and symlinks in busybox 1.28.2

[Natanael Copa]

Hi,

There is a serious regression in busybox 1.28.2 that breaks booting
alpine linux machines.

The problem comes from /bin/sh -> /bin/busybox symlink no longer getting copied 
with cpio -p.

To reproduce:

$ mkdir -p 1/bin 2
$ ln -s /bin/busybox 1/bin/sh
$ (cd 1 && echo "/bin/sh" | cpio -vdmp ../2)
bin/sh
1 blocks
$ find 2/
2/
2/bin

The /bin/sh symlink was silently ignored and this causes the
initramfs's #/bin/sh script fail at boot.

It was previously possible to work around it by setting
EXTRACT_UNSAFE_SYMLINKS=1 but this no longer works.

https://git.alpinelinux.org/cgit/aports/commit/main/mkinitfs?id=1b6a167de8ce02d69dc8a8c8f4638aefd27c0ebe

Downstream bug report:
https://bugs.alpinelinux.org/issues/8751

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

bug in busybox grep -F -w

[Natanael Copa]

Hi!

It looks like there is a bug in `busybox grep -F -w`:

$ echo "aa" | busybox grep -F -w "a"
aa

I would expect that to not output anything. GNU grep does not output
anything, nor does FreeBSD grep.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Removing -n option from udhcpc

[Natanael Copa]

On Tue, 6 Feb 2018 20:04:20 +0100
Denys Vlasenko  wrote:

> On Mon, Feb 5, 2018 at 8:10 PM, Daykin, Evan  wrote:
> > Good Afternoon,
> >
> >
> >
> > We have a lot of devices which were shipped running Busybox 1.20.2. We ran a
> > test to see what would happen in the event of a power failure. When power
> > was restored, the devices all booted much faster than the switches they were
> > connected to, so udhcpc gave up and exited,  
> 
> Please be more specific. They exited how? what are the messages from them?
> 
> > leaving the devices unable to
> > obtain a DHCP lease.  We tried setting “udhcpc_opts” to –background in the
> > interfaces file, to no avail. As we understand it, -n and –b are not
> > mutually exclusive, thus the client will still give up and exit after a few
> > attempts.  
> 
> Of course. If you gave -n to udhcpc, it will exit if lease is not obtained.
> If you don't want this, don't give it the -n option.

FWIW, we use this config in Alpine Linux:

CONFIG_IFUPDOWN_UDHCPC_CMD_OPTIONS="-b"

which make ifup add -b to the udhcpc call so it goes to background til
network comes up. If a user don't want this behavior he can disable it
with `udhcp_opts -n` in /etc/network/interfaces. For that this patch
is needed:
https://git.alpinelinux.org/cgit/aports/tree/main/busybox/0010-udhcpc-Don-t-background-if-n-is-given.patch

With that patch no rebuild of busybox is needed to reconfig the
behavior.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] unzip: fix regression on big-endian machines

[Natanael Copa]

On Thu, 27 Jul 2017 15:39:35 +0200
Denys Vlasenko <vda.li...@googlemail.com> wrote:

> Applied, thanks!

This should go to 1_27_stable too.

Thanks!

> 
> On Tue, Jul 25, 2017 at 8:44 PM, Natanael Copa <nc...@alpinelinux.org> wrote:
> > This fixes a regression which was introduced with commit 2a0867a5
> > ("unzip: optional support for bzip2 and lzma") and causes unzip to exit
> > with error when extracting archives:
> >
> >   unzip: unsupported method 2048
> >
> > Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
> > ---
> >  archival/unzip.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/archival/unzip.c b/archival/unzip.c
> > index bb39d954e..8ed9ae7d5 100644
> > --- a/archival/unzip.c
> > +++ b/archival/unzip.c
> > @@ -114,6 +114,7 @@ typedef union {
> >
> >  #define FIX_ENDIANNESS_ZIP(zip) \
> >  do { if (BB_BIG_ENDIAN) { \
> > +   (zip).fmt.method= SWAP_LE16((zip).fmt.method  ); \
> > (zip).fmt.crc32 = SWAP_LE32((zip).fmt.crc32   ); \
> > (zip).fmt.cmpsize   = SWAP_LE32((zip).fmt.cmpsize ); \
> > (zip).fmt.ucmpsize  = SWAP_LE32((zip).fmt.ucmpsize); \
> > --
> > 2.13.2
> >
> > ___
> > busybox mailing list
> > busybox@busybox.net
> > http://lists.busybox.net/mailman/listinfo/busybox  

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] miscutils/microcom: Fixed segfault

[Natanael Copa]

On Fri, 4 Aug 2017 02:02:36 +0200
Denys Vlasenko  wrote:

> On Thu, Aug 3, 2017 at 7:11 AM, Marian Buschsieweke
>  wrote:
> > microcom did not check if required parameter TTY is present. Thus,
> > bb_basename() was called with a NULL pointer if TTY was missing.
> > This commit adds the missing check.  
> 
> Fixed, thanks! (A bit differently)

It should got to 1_27_stable too.

Thanks!

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] unzip: fix regression on big-endian machines

[Natanael Copa]

This fixes a regression which was introduced with commit 2a0867a5
("unzip: optional support for bzip2 and lzma") and causes unzip to exit
with error when extracting archives:

  unzip: unsupported method 2048

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---
 archival/unzip.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/archival/unzip.c b/archival/unzip.c
index bb39d954e..8ed9ae7d5 100644
--- a/archival/unzip.c
+++ b/archival/unzip.c
@@ -114,6 +114,7 @@ typedef union {
 
 #define FIX_ENDIANNESS_ZIP(zip) \
 do { if (BB_BIG_ENDIAN) { \
+   (zip).fmt.method= SWAP_LE16((zip).fmt.method  ); \
(zip).fmt.crc32 = SWAP_LE32((zip).fmt.crc32   ); \
(zip).fmt.cmpsize   = SWAP_LE32((zip).fmt.cmpsize ); \
(zip).fmt.ucmpsize  = SWAP_LE32((zip).fmt.ucmpsize); \
-- 
2.13.2

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

missing branch and tag for release 1.27.0

[Natanael Copa]

Hi,

Congrats with the 1.27.0 release!

I just want to point out that the 1_27_stable branch is missing. This
means that there is a broken link for "git" on the front page:
https://git.busybox.net/busybox/tree/?h=1_27_stable

The git tag for 1.27.0 seems also to be missing.

Thanks!

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: udhcpc changes in 1.25.0 that didn't seem to be mentioned?

[Natanael Copa]

Hi,

This issue is affecting Alpine Linux too. From the setup script we will
start network in the background while user creates the root password.
However, udhcpc will not print information on the screen about the
lease which will overwrite the "Enter root password" prompt.

I tried to redirect output to /dev/null, but this does not work since
the information is printed to stderr.

If we redirect stderr we will not be able to display any real error
messages.

I think we will have to patch busybox for Alpine to fix this, but it
would be nice to not need maintain a fork. Are there any suggestions
how to fix this?

-nc


On Sat, 25 Jun 2016 06:48:14 +1000
"Michael D. Setzer II"  wrote:

> I did a comparison between the dhcpc.c of 1.25.0 and 1.24.2 and found 91 
> lines that are different. 
> 
> Most of the lines are changes for bb_info_msg to bb_error_msg that would 
> account for the output going to stderr instead of stdout??
> 
> The other lines are the message are seem to be changed to all lower case. 
> Previous version had the first letter in caps??
> 
> There are other changes that seem to be the ones documented, but not clear 
> on why the changes from bb_info_msg to bb_error_msg, or changing the text 
> of messages. 
> 
> Also, don't know if this is just with this program, or if it has been done 
> with all 
> busybox programs?
> 
> Thanks.
> 
> 
> On 24 Jun 2016 at 20:55, Michael D. Setzer II wrote:
> 
> From: "Michael D. Setzer II" 
> To:   busybox@busybox.net
> Date sent:Fri, 24 Jun 2016 20:55:06 +1000
> Subject:  udhcpc changes in 1.25.0 that didn't seem to be 
> mentioned?
> Priority: normal
> 
> > Downloaded and built the new busybox 1.25.0, and had a script that was no 
> > longer working? 
> > 
> > The original line was
> >  IP_ADDRESS=`udhcpc -n -t 8 -T 5 -i $FOUND_IF -s /udhcpc.sh | grep   
> > Lease | cut -d\  -f3`
> > 
> > But that no longer worked for two reasons? Seems the output is going to 
> > stderr instead of stdout? Second the case of Lease has changed to lease?
> > 
> > This is how I changed the lines in this little script that uses it or a 
> > version in 
> > several places. 
> > 
> >  IP_ADDRESS=`udhcpc -n -t 8 -T 5 -i $FOUND_IF -s /udhcpc.sh 2>&1 | grep 
> > -i  Lease | cut -d\  -f3`
> > 
> > This script is only 91 lines in length, but my larger script has about 
> > 2000+ 
> > lines, and it also has 3 of this type of lines.
> > 
> > In past, don't recall having any issues in upgrade busybox to the newer 
> > version? 
> > 
> > The script did actually setup the IP address, but this variable is used to 
> > determine if it got an IP, and displays an error message since it came up 
> > as 
> > blank without the modification.
> > 
> > Thanks.
> > 
> > +--+
> >   Michael D. Setzer II -  Computer Science Instructor  
> >   Guam Community College  Computer Center  
> >   mailto:mi...@kuentos.guam.net
> >   mailto:msetze...@gmail.com
> >   Guam - Where America's Day Begins
> >   G4L Disk Imaging Project maintainer 
> >   http://sourceforge.net/projects/g4l/
> > +--+
> > 
> > http://setiathome.berkeley.edu (Original)
> > Number of Seti Units Returned:  19,471
> > Processing time:  32 years, 290 days, 12 hours, 58 minutes
> > (Total Hours: 287,489)
> > 
> > BOINC@HOME CREDITS
> > ABC 16613838.513356   |   EINSTEIN
> > 101632185.95569545014114.055355
> > ROSETTA 45014114.055355   |   SETI84572139.973936
> > 
> > ___
> > busybox mailing list
> > busybox@busybox.net
> > http://lists.busybox.net/mailman/listinfo/busybox  
> 
> 
> +--+
>   Michael D. Setzer II -  Computer Science Instructor  
>   Guam Community College  Computer Center  
>   mailto:mi...@kuentos.guam.net
>   mailto:msetze...@gmail.com
>   Guam - Where America's Day Begins
>   G4L Disk Imaging Project maintainer 
>   http://sourceforge.net/projects/g4l/
> +--+
> 
> http://setiathome.berkeley.edu (Original)
> Number of Seti Units Returned:  19,471
> Processing time:  32 years, 290 days, 12 hours, 58 minutes
> (Total Hours: 287,489)
> 
> BOINC@HOME CREDITS
> ABC 16613838.513356   |   EINSTEIN
> 102570568.45569545339942.544286
> ROSETTA 45339942.544286   |   SETI85273027.442117
> 
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] add/remove-shell: prevent world writable /etc/shells

[Natanael Copa]

add-shell will not preserve the current permissions, and if umask is 0
it will create the /etc/shells world writable. To reproduce:

  umask 0; add-shell /bin/bash; ls -l /etc/shells

As a workaround we add the current st_mode with xopen3, which at least
will prevent /etc/shells to get more permissions than it previously
had.

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---
 loginutils/add-remove-shell.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/loginutils/add-remove-shell.c b/loginutils/add-remove-shell.c
index af7c31779..a434d054d 100644
--- a/loginutils/add-remove-shell.c
+++ b/loginutils/add-remove-shell.c
@@ -54,6 +54,7 @@ int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
FILE *orig_fp;
char *orig_fn;
char *new_fn;
+   struct stat sb;
 
argv++;
 
@@ -63,6 +64,7 @@ int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
orig_fp = fopen_for_read(orig_fn);
 
new_fn = xasprintf("%s.tmp", orig_fn);
+   xfstat(fileno(orig_fp), , orig_fn);
/*
 * O_TRUNC or O_EXCL? At the first glance, O_EXCL looks better,
 * since it prevents races. But: (1) it requires a retry loop,
@@ -71,14 +73,7 @@ int add_remove_shell_main(int argc UNUSED_PARAM, char **argv)
 * after which it should revert to O_TRUNC.
 * For now, I settle for O_TRUNC instead.
 */
-   xmove_fd(xopen(new_fn, O_WRONLY | O_CREAT | O_TRUNC), STDOUT_FILENO);
-
-   /* TODO:
-   struct stat sb;
-   xfstat(fileno(orig_fp), );
-   xfchown(STDOUT_FILENO, sb.st_uid, sb.st_gid);
-   xfchmod(STDOUT_FILENO, sb.st_mode);
-   */
+   xmove_fd(xopen3(new_fn, O_WRONLY | O_CREAT | O_TRUNC, sb.st_mode), 
STDOUT_FILENO);
 
if (orig_fp) {
/* Copy old file, possibly skipping removed shell names */
-- 
2.12.2

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] ntpd: improve postponed hostname resolution

[Natanael Copa]

Run the namelookup from the main loop so a misspelled first ntp server
name does not block everything forever.

This fixes the following situation which would block forever:
  $ sudo ./busybox ntpd -dn -p foobar  -p pool.ntp.org
  ntpd: bad address 'foobar'
  ntpd: bad address 'foobar'
  ntpd: bad address 'foobar'
  ...

This patch is based on Kaarle Ritvanens work.
http://lists.busybox.net/pipermail/busybox/2016-May/084197.html

function old new   delta
static.set_next-  23 +23
step_time426 422  -4
recv_and_process_peer_pkt   23422338  -4
ntpd_main   11781171  -7
add_peers229 217 -12
resolve_peer_hostname102  88 -14
.rodata   125081  125065 -16
--
(add/remove: 1/0 grow/shrink: 0/6 up/down: 23/-57)Total: -34
bytes
   textdata bss dec hex filename
 802129   154592192  819780   c8244 busybox_old
 802095   154592192  819746   c8222 busybox_unstripped

Signed-off-by: Kaarle Ritvanen <kaarle.ritva...@datakunkku.fi>
Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---
 networking/ntpd.c | 71 ++-
 1 file changed, 33 insertions(+), 38 deletions(-)

diff --git a/networking/ntpd.c b/networking/ntpd.c
index b7fa5dce9..97e3919fb 100644
--- a/networking/ntpd.c
+++ b/networking/ntpd.c
@@ -155,6 +155,7 @@
 #define RETRY_INTERVAL32/* on send/recv error, retry in N secs (need 
to be power of 2) */
 #define NOREPLY_INTERVAL 512/* sent, but got no reply: cap next query by 
this many seconds */
 #define RESPONSE_INTERVAL 16/* wait for reply up to N secs */
+#define HOSTNAME_INTERVAL  4/* hostname lookup failed. Wait N secs for 
next try */
 
 /* Step threshold (sec). std ntpd uses 0.128.
  */
@@ -293,6 +294,7 @@ typedef struct {
 
 typedef struct {
len_and_sockaddr *p_lsa;
+   char *p_hostname;
char *p_dotted;
int  p_fd;
int  datapoint_idx;
@@ -318,7 +320,6 @@ typedef struct {
datapoint_t  filter_datapoint[NUM_DATAPOINTS];
/* last sent packet: */
msg_tp_xmt_msg;
-   char p_hostname[1];
 } peer_t;
 
 
@@ -791,27 +792,17 @@ reset_peer_stats(peer_t *p, double offset)
 }
 
 static void
-resolve_peer_hostname(peer_t *p, int loop_on_fail)
-{
-   len_and_sockaddr *lsa;
-
- again:
-   lsa = host2sockaddr(p->p_hostname, 123);
-   if (!lsa) {
-   /* error message already emitted by host2sockaddr() */
-   if (!loop_on_fail)
-   return;
-//FIXME: do this to avoid infinite looping on typo in a hostname?
-//well... in which case, what is a good value for loop_on_fail?
-   //if (--loop_on_fail == 0)
-   //  xfunc_die();
-   sleep(5);
-   goto again;
+resolve_peer_hostname(peer_t *p) {
+   len_and_sockaddr *lsa = host2sockaddr(p->p_hostname, 123);
+   if (lsa) {
+   if (p->p_lsa) {
+   free(p->p_lsa);
+   free(p->p_dotted);
+   }
+   p->p_lsa = lsa;
+   p->p_dotted = xmalloc_sockaddr2dotted_noport(>u.sa);
}
-   free(p->p_lsa);
-   free(p->p_dotted);
-   p->p_lsa = lsa;
-   p->p_dotted = xmalloc_sockaddr2dotted_noport(>u.sa);
+   set_next(p, lsa ? 0 : HOSTNAME_INTERVAL);
 }
 
 static void
@@ -820,28 +811,29 @@ add_peers(const char *s)
llist_t *item;
peer_t *p;
 
-   p = xzalloc(sizeof(*p) + strlen(s));
-   strcpy(p->p_hostname, s);
-   resolve_peer_hostname(p, /*loop_on_fail=*/ 1);
+   p = xzalloc(sizeof(*p));
+   p->p_hostname = xstrdup(s);
+   resolve_peer_hostname(p);
 
/* Names like N..pool.ntp.org are randomly resolved
 * to a pool of machines. Sometimes different N's resolve to the same 
IP.
 * It is not useful to have two peers with same IP. We skip duplicates.
 */
-   for (item = G.ntp_peers; item != NULL; item = item->link) {
-   peer_t *pp = (peer_t *) item->data;
-   if (strcmp(p->p_dotted, pp->p_dotted) == 0) {
-   bb_error_msg("duplicate peer %s (%s)", s, p->p_dotted);
-   free(p->p_lsa);
-   free(p->p_dotted);
-   free(p);
-   return;
+   if (p->p_lsa)
+   for (item = G.ntp_peers; item != NULL; item = item->link

Re: ash regression in busybox-1.26.1

[Natanael Copa]

On Tue, 3 Jan 2017 16:41:03 +0100
Denys Vlasenko <vda.li...@googlemail.com> wrote:

> Any chance you can narrow it down?

Yeah, i figured it is the glob implementation in musl that causes
problems. I changed the config setting to use ash internal glob.

Sorry,

Thanks!

-nc

> 
> On Tue, Jan 3, 2017 at 3:41 PM, Natanael Copa <nc...@alpinelinux.org> wrote:
> > Hi
> >
> > There is a regression in ash in busybox-1.26.1, with
> > https://git.busybox.net/busybox/commit/?id=ea7d2f6ec0596789fc5b2e3fca3b7a602bfa2c26
> >  applied.
> >
> > To reproduce, try build flac-1.3.2 with CONFIG_SHELL=/bin/ash
> >
> >
> > ...
> > Making all in libFLAC++
> > make[3]: Entering directory 
> > '/home/ncopa/aports/main/flac/src/flac-1.3.2/src/libFLAC++'
> >   CXX  metadata.lo
> >   CXX  stream_decoder.lo
> >   CXX  stream_encoder.lo
> > ../../libtool: eval: line 1: out of memory
> > ../../libtool: make[3]: *** [Makefile:528: metadata.lo] Error 2
> > eval: line 1: out of memory
> > make[3]: *** Waiting for unfinished jobs
> >
> >
> >
> > This happens with musl-1.1.16, also with 
> > https://git.musl-libc.org/cgit/musl/commit/?id=769f53598e781ffc89191520f3f8a93cb58db91f
> >  applied.
> >
> >
> > -nc
> > ___
> > busybox mailing list
> > busybox@busybox.net
> > http://lists.busybox.net/mailman/listinfo/busybox  

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

ash regression in busybox-1.26.1

[Natanael Copa]

Hi

There is a regression in ash in busybox-1.26.1, with
https://git.busybox.net/busybox/commit/?id=ea7d2f6ec0596789fc5b2e3fca3b7a602bfa2c26
 applied.

To reproduce, try build flac-1.3.2 with CONFIG_SHELL=/bin/ash


...
Making all in libFLAC++
make[3]: Entering directory 
'/home/ncopa/aports/main/flac/src/flac-1.3.2/src/libFLAC++'
  CXX  metadata.lo
  CXX  stream_decoder.lo
  CXX  stream_encoder.lo
../../libtool: eval: line 1: out of memory
../../libtool: make[3]: *** [Makefile:528: metadata.lo] Error 2
eval: line 1: out of memory
make[3]: *** Waiting for unfinished jobs



This happens with musl-1.1.16, also with 
https://git.musl-libc.org/cgit/musl/commit/?id=769f53598e781ffc89191520f3f8a93cb58db91f
 applied.


-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: running `busybox --install -s` as non-root in a staging dir

[Natanael Copa]

On Thu, 15 Dec 2016 16:37:22 +0100
Natanael Copa <nc...@alpinelinux.org> wrote:

> Hi,
> 
> I am working on creating a rootfs tarball for alpine linux to be used
> as base for things like docker images. Currently all alpine release
> iso images are created as non-root, with some help from fakeroot.
> 
> I do have a problem with the rootfs image. The problem is that package
> manager (apk-tools) runs the install scripts in a chroot. This is
> because we need `/bin/busybox --install -s` to run relative the temp
> root. chroot(2) will fail in fakeroot with permission denied.
> 
> I need a way to install the busybox symlinks relative a root. I know
> `/bin/busybox --install -s ` but that will install all the
> symlinks in a give dir instead of /usr/bin /sbin etc.
> 
> Possible solutions:
> 
> 1) Add a -p flag to indicate that DIR is a root prefix instead of
>target directory for all links.
> 
> /bin/busybox --install -s -p DIR
> 
> 2) Add an flag to print all install paths to the applet.
> 
>/bin/busybox --print-paths

I just relalized there is a `/bin/busybox --list-full`. I think I am all good.

Thanks!

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

running `busybox --install -s` as non-root in a staging dir

[Natanael Copa]

Hi,

I am working on creating a rootfs tarball for alpine linux to be used
as base for things like docker images. Currently all alpine release
iso images are created as non-root, with some help from fakeroot.

I do have a problem with the rootfs image. The problem is that package
manager (apk-tools) runs the install scripts in a chroot. This is
because we need `/bin/busybox --install -s` to run relative the temp
root. chroot(2) will fail in fakeroot with permission denied.

I need a way to install the busybox symlinks relative a root. I know
`/bin/busybox --install -s ` but that will install all the
symlinks in a give dir instead of /usr/bin /sbin etc.

Possible solutions:

1) Add a -p flag to indicate that DIR is a root prefix instead of
   target directory for all links.

/bin/busybox --install -s -p DIR

2) Add an flag to print all install paths to the applet.

   /bin/busybox --print-paths

I want avoid a custom patch for busybox, so what would be an acceptable
solution for upstream busybox? Other ideas?

Thanks!

-nc

PS. There is a fakechroot but it appears to be very gnu libc centric and
does not build with musl libc.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: bug in ash set -e

[Natanael Copa]

On Wed, 09 Nov 2016 16:12:41 +
Ron Yorston <r...@pobox.com> wrote:

> Natanael Copa wrote:
> >I have found a bug (or weird feature?) in busybox ash:
> >
> >$ dash -c "set -e ; ( false ) ; echo 'should not echo'"
> >$ bash -c "set -e ; ( false ) ; echo 'should not echo'"
> >$ ash -c "set -e ; ( false ) ; echo 'should not echo'"
> >should not echo
> >$
> >
> >It looks like `set -e` does not catch error in ( )  
> 
> This is true in 1.25.1.  It's fixed in master by this:
> 
> commit 204c7fb2293f67f6277f917e854188f5540e6955
> Author: Rostislav Skudnov <rostis...@tuxera.com>
> Date:   Fri Sep 16 19:04:02 2016 +
> 
> ash: exit after subshell error when errexit option is set
> 
> When "set -e" option is on, shell must exit when any command fails,
> including compound commands of the form (compound-list) executed in a
> subshell. Bash and dash shells have this behaviour.
> 
> Also add a corresponding testcase.
> 
> Ron

So it should be added to fixes-1.25.1/ and inclueded in 1.25.2

Thanks!

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

bug in ash set -e

[Natanael Copa]

Hi,

I have found a bug (or weird feature?) in busybox ash:

$ dash -c "set -e ; ( false ) ; echo 'should not echo'"
$ bash -c "set -e ; ( false ) ; echo 'should not echo'"
$ ash -c "set -e ; ( false ) ; echo 'should not echo'"
should not echo
$

It looks like `set -e` does not catch error in ( )

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] iproute: ensure scope is correctly initialised

[Natanael Copa]

On Fri, 05 Aug 2016 19:39:31 +0100
Ron Yorston <r...@pobox.com> wrote:

> Code in iproute.c attempted to avoid assigning values to structure
> elements which were know to be zero unless the value to be assigned
> was non-zero.
> 
> Commit ce4bc1ed added some more such cases.  However, the treatment
> of req.r.rtm_scope was incorrect.  Although this saved a few bytes it
> resulted in incorrect behaviour like:
> 
>$ sudo busybox ip route add 192.168.0.0/24 via 10.98.106.9 dev wlan0
>ip: RTNETLINK answers: Invalid argument
> 
> In practice this attempt at optimisation results in no saving, so
> remove it.
> 
> Reported-by: Natanael Copa <nc...@alpinelinux.org>
> Signed-off-by: Ron Yorston <r...@pobox.com>

Acked-by: Natanael Copa <nc...@alpinelinux.org>

We now use this in alpine linux and it works.

Thanks!

> ---
>  networking/libiproute/iproute.c | 15 +--
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/networking/libiproute/iproute.c b/networking/libiproute/iproute.c
> index e674e9a..8db9aef 100644
> --- a/networking/libiproute/iproute.c
> +++ b/networking/libiproute/iproute.c
> @@ -356,18 +356,13 @@ IF_FEATURE_IP_RULE(ARG_table,)
>   req.n.nlmsg_flags = NLM_F_REQUEST | flags;
>   req.n.nlmsg_type = cmd;
>   req.r.rtm_family = preferred_family;
> - if (RT_TABLE_MAIN != 0) /* if it is zero, memset already did it */
> - req.r.rtm_table = RT_TABLE_MAIN;
> - if (RT_SCOPE_NOWHERE != 0)
> - req.r.rtm_scope = RT_SCOPE_NOWHERE;
> + req.r.rtm_table = RT_TABLE_MAIN;
> + req.r.rtm_scope = RT_SCOPE_NOWHERE;
>  
>   if (cmd != RTM_DELROUTE) {
> - if (RTPROT_BOOT != 0)
> - req.r.rtm_protocol = RTPROT_BOOT;
> - if (RT_SCOPE_UNIVERSE != 0)
> - req.r.rtm_scope = RT_SCOPE_UNIVERSE;
> - if (RTN_UNICAST != 0)
> - req.r.rtm_type = RTN_UNICAST;
> + req.r.rtm_protocol = RTPROT_BOOT;
> + req.r.rtm_scope = RT_SCOPE_UNIVERSE;
> + req.r.rtm_type = RTN_UNICAST;
>   }
>  
>   mxrta->rta_type = RTA_METRICS;

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

ip route add bug in 1.25.0

[Natanael Copa]

Hi,

  ip route add 0.0.0.0/0 via  dev eth0


This results in default route not being set which results in broken
network in alpine linux. This worked just fine in busybox-1.24.x

The test case I have been using in my local network is:

$ sudo busybox ip route add 192.168.0.0/24 via 10.98.106.9 dev wlan0
ip: RTNETLINK answers: Invalid argument

With strace:


$ sudo strace busybox ip route add 192.168.0.0/24 via 10.98.106.9 dev wlan0
execve("/bin/busybox", ["busybox", "ip", "route", "add", "192.168.0.0/24", 
"via", "10.98.106.9", "dev", "wlan0"], [/* 18 vars */]) = 0
arch_prctl(ARCH_SET_FS, 0x685076d07c48) = 0
set_tid_address(0x685076d07c80) = 6247
mprotect(0x685076d04000, 4096, PROT_READ) = 0
mprotect(0x1f855b2d000, 16384, PROT_READ) = 0
getuid()= 0
socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE) = 3
bind(3, {sa_family=AF_NETLINK, pid=0, groups=}, 12) = 0
getsockname(3, {sa_family=AF_NETLINK, pid=6247, groups=}, [12]) = 0
sendto(3, "\24\0\0\0\22\0\1\3\357\227\244W\0\0\0\0\0\2515X", 20, 0, 
{sa_family=AF_NETLINK, pid=0, groups=}, 12) = 20
brk(NULL)   = 0x1f8589d7f20
brk(0x1f8589db000)  = 0x1f8589db000
recvmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=}, 
msg_iov(1)=[{"\234\4\0\0\20\0\2\0\357\227\244Wg\30\0\0\0\0\4\3\1\0\0\0I\0\1\0\0\0\0\0"...,
 8192}], msg_controllen=0, msg_flags=0}, 0) = 3540
recvmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=}, 
msg_iov(1)=[{"\240\4\0\0\20\0\2\0\357\227\244Wg\30\0\0\0\0\1\0\4\0\0\0\2\20\0\0\0\0\0\0"...,
 8192}], msg_controllen=0, msg_flags=0}, 0) = 4192
recvmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=}, 
msg_iov(1)=[{"\24\0\0\0\3\0\2\0\357\227\244Wg\30\0\0\0\0\0\0", 8192}], 
msg_controllen=0, msg_flags=0}, 0) = 20
socket(AF_LOCAL, SOCK_DGRAM|SOCK_CLOEXEC, 0) = 4
ioctl(4, SIOCGIFINDEX, {ifr_name="wlan0", }) = 0
close(4)= 0
sendmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=}, 
msg_iov(1)=[{"4\0\0\0\30\0\5\6\360\227\244W\0\0\0\0\2\30\0\0\376\3\377\1\0\0\0\0\10\0\1\0"...,
 52}], msg_controllen=0, msg_flags=0}, 0) = 52
recvmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=}, 
msg_iov(1)=[{"H\0\0\0\2\0\0\0\360\227\244Wg\30\0\0\352\377\377\3774\0\0\0\30\0\5\6\360\227\244W"...,
 8192}], msg_controllen=0, msg_flags=0}, 0) = 72
write(2, "ip: RTNETLINK answers: Invalid a"..., 40ip: RTNETLINK answers: 
Invalid argument
) = 40
exit_group(2)   = ?
+++ exited with 2 +++




I didn't have time to investigate it futher, but will continue as soon
I get time, but decided to post it here in case someone beats me to it.

Thanks

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] lineedit: trivial codeshrink for vi-mode

[Natanael Copa]

Denys,

Any reason to not apply this?

-nc

On Fri, 29 May 2015 10:08:56 +0200
Natanael Copa <nc...@alpinelinux.org> wrote:

> Introduce and use BB_isalnum_or_underscore().
> 
> bloatcheck on x86_64:
> function old new   delta
> BB_isalnum_or_underscore   -  46 +46
> vi_end_motion176 166 -10
> vi_back_motion   169 155 -14
> vi_word_motion   190 172 -18
> BB_isalnum39   - -39
> --
> (add/remove: 1/1 grow/shrink: 0/3 up/down: 46/-81)Total: -35
> bytes
> 
> Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
> ---
>  libbb/lineedit.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/libbb/lineedit.c b/libbb/lineedit.c
> index a83e07c..507b829 100644
> --- a/libbb/lineedit.c
> +++ b/libbb/lineedit.c
> @@ -81,6 +81,9 @@
>  static bool BB_isspace(CHAR_T c) { return ((unsigned)c < 256 && isspace(c)); 
> }
>  # if ENABLE_FEATURE_EDITING_VI
>  static bool BB_isalnum(CHAR_T c) { return ((unsigned)c < 256 && isalnum(c)); 
> }
> +static bool BB_isalnum_or_underscore(CHAR_T c) {
> + return (BB_isalnum(c) || c == '_');
> +}
>  # endif
>  static bool BB_ispunct(CHAR_T c) { return ((unsigned)c < 256 && ispunct(c)); 
> }
>  # undef isspace
> @@ -1572,9 +1575,9 @@ vi_word_motion(int eat)
>  {
>   CHAR_T *command = command_ps;
>  
> - if (BB_isalnum(command[cursor]) || command[cursor] == '_') {
> + if (BB_isalnum_or_underscore(command[cursor])) {
>   while (cursor < command_len
> -  && (BB_isalnum(command[cursor+1]) || command[cursor+1] == '_')
> +  && (BB_isalnum_or_underscore(command[cursor+1]))
>   ) {
>   input_forward();
>   }
> @@ -1616,9 +1619,9 @@ vi_end_motion(void)
>   input_forward();
>   if (cursor >= command_len-1)
>   return;
> - if (BB_isalnum(command[cursor]) || command[cursor] == '_') {
> + if (BB_isalnum_or_underscore(command[cursor])) {
>   while (cursor < command_len-1
> -  && (BB_isalnum(command[cursor+1]) || command[cursor+1] == '_')
> +  && (BB_isalnum_or_underscore(command[cursor+1]))
>   ) {
>   input_forward();
>   }
> @@ -1651,9 +1654,9 @@ vi_back_motion(void)
>   input_backward(1);
>   if (cursor <= 0)
>   return;
> - if (BB_isalnum(command[cursor]) || command[cursor] == '_') {
> + if (BB_isalnum_or_underscore(command[cursor])) {
>   while (cursor > 0
> -  && (BB_isalnum(command[cursor-1]) || command[cursor-1] == '_')
> +  && (BB_isalnum_or_underscore(command[cursor-1]))
>   ) {
>   input_backward(1);
>   }

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 1/2] gzip: fix compression level bug. Closes 9131

[Natanael Copa]

fix broken logic to get the gzip_level_config value from options -1 to
-9.

This fixes an off-by-one bug that caused gzip -9 output bigger files
than the other compression levels.

It fixes so that compression level 1 to 3 are actually mapped to level 4
as comments say.

It also fixes that levels -4 to -9 is mapped to correct level and avoids
out-of-bounds access.

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---

See https://bugs.busybox.net/show_bug.cgi?id=9131 for more details

 archival/gzip.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/archival/gzip.c b/archival/gzip.c
index 8f1e4ff..15ba57c 100644
--- a/archival/gzip.c
+++ b/archival/gzip.c
@@ -2220,10 +2220,7 @@ int gzip_main(int argc UNUSED_PARAM, char **argv)
opt >>= ENABLE_GUNZIP ? 7 : 5; /* drop cfv[dt]qn bits */
if (opt == 0)
opt = 1 << 6; /* default: 6 */
-   /* Map 1..3 to 4 */
-   if (opt & 0x7)
-   opt |= 1 << 4;
-   opt = ffs(opt >> 3);
+   opt = ffs(opt >> 4); /* Maps 1..3 to 4 */
max_chain_length = 1 << gzip_level_config[opt].chain_shift;
good_match   = gzip_level_config[opt].good;
max_lazy_match   = gzip_level_config[opt].lazy2 * 2;
-- 
2.9.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 2/2] gzip: add test that checks that -9 compresses better than -1

[Natanael Copa]

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---
 testsuite/gzip/gzip-compression-levels | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 testsuite/gzip/gzip-compression-levels

diff --git a/testsuite/gzip/gzip-compression-levels 
b/testsuite/gzip/gzip-compression-levels
new file mode 100644
index 000..36289e6
--- /dev/null
+++ b/testsuite/gzip/gzip-compression-levels
@@ -0,0 +1,3 @@
+level1=$(busybox gzip -c -1 $(which busybox) | wc -c)
+level9=$(busybox gzip -c -9 $(which busybox) | wc -c)
+test $level1 -gt $level9
-- 
2.9.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: busybox 1.25 ntpd retry initial DNS resolution (forever, no timeout for now).

[Natanael Copa]

On Tue, 28 Jun 2016 20:19:29 +0200
"KP.Kirchdoerfer"  wrote:

> Hi;
> 
> I'm testing busybox 1.25 and wonder how to deal with the commit 
> 
> https://git.busybox.net/busybox/commit/networking?id=e4caf1dd9ce8569371a0eeb77ccf02a572dc0f11
> 
> At the first look it seems to be an obvious improvement, but then I'm afraid 
> it 
> may generate a hard to resolve problem.
> 
> I start ntpd by default from /etc/init.d
> 
> There might be no working network connection (not configured properly for 
> whatever reason, hardware problems, whatelse).
> 
> With busybox 1.24 ntpd fails to start and the boot process continues up to a 
> shell login  - allowing me to configure, start diagnostics and resolve errors.
> 
> With busybox 1.25 ntpd seems to loop forever if now NTP servers are found, 
> blocking the boot process  and I never get a login to solve a possible pb or 
> to do a first time configuration.
> 
> Any hints how this can be solved?

It looks like the original implementation from Kaarle[1] handled it better.

Also, if you add multiple ntp servers and the first is broken, then
will ntpd never set time. Kaarle's implementation would just skip the
bad server and use the working. To test:

  ./busybox ntpd -d -n -p qwe.rty.ghj.kl -p 0.no.pool.ntp.org


And finally, bloat check on x86_64 suggests that Kaarles implementation
is -41 bytes compared to current git master.



[1]: http://lists.busybox.net/pipermail/busybox/2016-May/084197.html

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] df: use f_frsize instead of f_bsize for correct sizes

[Natanael Copa]

Use the correct field  f_frsize instead of f_bsize.

The statfs f_bsize is the "Optimal transfer block size" while the
f_frsize is the "Fragment size (since Linux 2.6)". On some FUSE
filesystems those may differ.

Fixes bug 9046

URL: https://bugs.busybox.net/show_bug.cgi?id=9046
Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---
 coreutils/df.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/coreutils/df.c b/coreutils/df.c
index d79c11a..06b2920 100644
--- a/coreutils/df.c
+++ b/coreutils/df.c
@@ -188,7 +188,7 @@ int df_main(int argc UNUSED_PARAM, char **argv)
if (opt & OPT_INODE) {
s.f_blocks = s.f_files;
s.f_bavail = s.f_bfree = s.f_ffree;
-   s.f_bsize = 1;
+   s.f_frsize = 1;
 
if (df_disp_hr)
df_disp_hr = 1;
@@ -246,26 +246,26 @@ int df_main(int argc UNUSED_PARAM, char **argv)
 
 #if ENABLE_FEATURE_HUMAN_READABLE
printf(" %9s ",
-   /* f_blocks x f_bsize / df_disp_hr, show one 
fractional,
+   /* f_blocks x f_frsize / df_disp_hr, show one 
fractional,
 * use suffixes if df_disp_hr == 0 */
-   make_human_readable_str(s.f_blocks, s.f_bsize, 
df_disp_hr));
+   make_human_readable_str(s.f_blocks, s.f_frsize, 
df_disp_hr));
 
printf(" %9s " + 1,
-   /* EXPR x f_bsize / df_disp_hr, show one 
fractional,
+   /* EXPR x f_frsize / df_disp_hr, show one 
fractional,
 * use suffixes if df_disp_hr == 0 */
make_human_readable_str((s.f_blocks - 
s.f_bfree),
-   s.f_bsize, df_disp_hr));
+   s.f_frsize, df_disp_hr));
 
printf("%9s %3u%% %s\n",
-   /* f_bavail x f_bsize / df_disp_hr, show one 
fractional,
+   /* f_bavail x f_frsize / df_disp_hr, show one 
fractional,
 * use suffixes if df_disp_hr == 0 */
-   make_human_readable_str(s.f_bavail, s.f_bsize, 
df_disp_hr),
+   make_human_readable_str(s.f_bavail, s.f_frsize, 
df_disp_hr),
blocks_percent_used, mount_point);
 #else
printf(" %9lu %9lu %9lu %3u%% %s\n",
-   kscale(s.f_blocks, s.f_bsize),
-   kscale(s.f_blocks - s.f_bfree, s.f_bsize),
-   kscale(s.f_bavail, s.f_bsize),
+   kscale(s.f_blocks, s.f_frsize),
+   kscale(s.f_blocks - s.f_bfree, s.f_frsize),
+   kscale(s.f_bavail, s.f_frsize),
blocks_percent_used, mount_point);
 #endif
}
-- 
2.9.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH v2] libbb: fix time parsing of [[CC]YY]MMDDhhmm[.SS]

[Natanael Copa]

If SS is not given a value, it is assumed to be zero.
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/touch.html

closes 8951

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---
V1 -> V2 changes:

 - always set seconds to 0. If you for example do: `touch -t 1100 somefile`
   you will expect to set timestamp to 11:00:00 AM not, 11:00:

 libbb/time.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libbb/time.c b/libbb/time.c
index aa19a47..82e6cb1 100644
--- a/libbb/time.c
+++ b/libbb/time.c
@@ -186,6 +186,7 @@ void FAST_FUNC parse_datestr(const char *date_str, struct 
tm *ptm)
} else {
bb_error_msg_and_die(bb_msg_invalid_date, date_str);
}
+   ptm->tm_sec = 0; /* assume zero if [.SS] is not given */
if (end == '.') {
/* xxx.SS */
if (sscanf(strchr(date_str, '.') + 1, "%u%c",
-- 
2.8.2

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] libbb: fix time parsing of [[CC]YY]MMDDhhmm[.SS]

[Natanael Copa]

If SS is not given a value, it is assumed to be zero.
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/touch.html

closes 8951

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---
 libbb/time.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libbb/time.c b/libbb/time.c
index aa19a47..44dd4aa 100644
--- a/libbb/time.c
+++ b/libbb/time.c
@@ -159,6 +159,8 @@ void FAST_FUNC parse_datestr(const char *date_str, struct 
tm *ptm)
>tm_hour,
>tm_min,
) >= 5) {
+   /* assume zero if [.SS] is not given */
+   ptm->tm_sec = 0;
/* Adjust month from 1-12 to 0-11 */
ptm->tm_mon -= 1;
if ((int)cur_year >= 50) { /* >= 1950 */
@@ -181,6 +183,8 @@ void FAST_FUNC parse_datestr(const char *date_str, struct 
tm *ptm)
>tm_hour,
>tm_min,
) >= 5) {
+   /* assume zero if [.SS] is not given */
+   ptm->tm_sec = 0;
ptm->tm_year -= 1900; /* Adjust years */
ptm->tm_mon -= 1; /* Adjust month from 1-12 to 0-11 */
} else {
-- 
2.8.2

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] loginutils: add portable groupadd applet

[Natanael Copa]

Add groupadd applet which aims to be portable with groupadd from
shadow-utils/pkg-shadow. groupadd exists in both debian/ubuntu and
fedora/centos while addgroup only exists on debian/ubuntu.

We add the groupadd applet to improve portability, while keeping
addgroup to be backwards compatible.

function old new   delta
new_group  - 214+214
groupadd_main  - 166+166
.rodata93576   93643 +67
packed_usage   19378   19413 +35
groupadd_longopts  -  16 +16
applet_names16321641  +9
applet_main 19841992  +8
bbconfig_config_bz2 55435544  +1
applet_suid   62  63  +1
applet_install_loc   124 125  +1
addgroup_main503 303-200
--
(add/remove: 3/0 grow/shrink: 7/1 up/down: 518/-200)  Total: 318
bytes
   textdata bss dec hex filename
 593356   114433096  607895   94697 busybox_old
 593627   114513096  608174   947ae busybox_unstripped

Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---

I kept the addgroup applet for the following reasons:
 - We may need the backward compatible addgroup for a migration period to
   not break things.
 - the `shadow` groupadd can not add users to a group. We need keep addgroup
   til we have an applet that can add users to a group.

 libbb/Kbuild.src  |  1 +
 loginutils/addgroup.c | 69 ++-
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/libbb/Kbuild.src b/libbb/Kbuild.src
index b08ce11..b2a5fdc 100644
--- a/libbb/Kbuild.src
+++ b/libbb/Kbuild.src
@@ -140,6 +140,7 @@ lib-$(CONFIG_ADDGROUP) += update_passwd.o
 lib-$(CONFIG_ADDUSER) += update_passwd.o
 lib-$(CONFIG_DELGROUP) += update_passwd.o
 lib-$(CONFIG_DELUSER) += update_passwd.o
+lib-$(CONFIG_GROUPADD) += update_passwd.o
 
 lib-$(CONFIG_FTPD) += correct_password.o
 lib-$(CONFIG_PASSWD) += pw_encrypt.o update_passwd.o obscure.o
diff --git a/loginutils/addgroup.c b/loginutils/addgroup.c
index 4d4fc3f..bc0d56c 100644
--- a/loginutils/addgroup.c
+++ b/loginutils/addgroup.c
@@ -31,9 +31,25 @@
 //config:addgroup will add an existing user to an
 //config:existing group.
 
+//config:config GROUPADD
+//config:  bool "groupadd"
+//config:  default n
+//config:  help
+//config:A more portable utility for creating a new group account.
+//config:
+//config:config FEATURE_GROUPADD_LONG_OPTIONS
+//config:  bool "Enable long options"
+//config:  default y
+//config:  depends on GROUPADD && LONG_OPTS
+//config:  help
+//config:Support long options for the groupadd applet.
+//config:
+
 //applet:IF_ADDGROUP(APPLET(addgroup, BB_DIR_USR_SBIN, BB_SUID_DROP))
+//applet:IF_GROUPADD(APPLET(groupadd, BB_DIR_USR_SBIN, BB_SUID_DROP))
 
 //kbuild:lib-$(CONFIG_ADDGROUP) += addgroup.o
+//kbuild:lib-$(CONFIG_GROUPADD) += addgroup.o
 
 //usage:#define addgroup_trivial_usage
 //usage:   "[-g GID] [-S] " IF_FEATURE_ADDUSER_TO_GROUP("[USER] ") "GROUP"
@@ -42,6 +58,13 @@
 //usage: "\n   -g GID  Group id"
 //usage: "\n   -S  Create a system group"
 
+//usage:#define groupadd_trivial_usage
+//usage:   "[-g GID] [-r] GROUP"
+//usage:#define groupadd_full_usage "\n\n"
+//usage:   "Add a group\n"
+//usage: "\n   -g GID  Group id"
+//usage: "\n   -r  Create a system group"
+
 #include "libbb.h"
 
 #if CONFIG_LAST_SYSTEM_ID < CONFIG_FIRST_SYSTEM_ID
@@ -101,6 +124,8 @@ static void new_group(char *group, gid_t gid)
struct group gr;
char *p;
 
+   die_if_bad_username(group);
+
/* make sure gid and group haven't already been allocated */
gr.gr_gid = gid;
gr.gr_name = group;
@@ -147,6 +172,7 @@ static const char addgroup_longopts[] ALIGN1 =
  * If called with two non-option arguments, addgroup
  * will add an existing user to an existing group.
  */
+#if ENABLE_ADDGROUP
 int addgroup_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int addgroup_main(int argc UNUSED_PARAM, char **argv)
 {
@@ -200,9 +226,50 @@ int addgroup_main(int argc UNUSED_PARAM, char **argv)
} else
 #endif /* ENABLE_FEATURE_ADDUSER_TO_GROUP */
{
-   die_if_bad_username(argv[0]);
new_group(argv[0], xatou_range(gid, 0, CONFIG_LAST_ID));
}
  

Re: [PATCH 1/2 1.24] ash: fix error during recursive processing of here document

[Natanael Copa]

On Tue, 22 Mar 2016 09:21:24 +
Ron Yorston  wrote:

> Mike Frysinger wrote:
> >i've cherry picked back this commit to the 1.24 branch  
> 
> Thanks, but why not 2/2 also?

I wonder that too. The 2/2 patch fixes this:

foo () {
cat 

Re: [Bug 6182] nologin: new applet

[Natanael Copa]

On Mon, 15 Feb 2016 19:09:20 -0500
Mike Frysinger <vap...@gentoo.org> wrote:

> On 15 Feb 2016 17:23, Natanael Copa wrote:
> > On Sat, 13 Feb 2016 13:32:10 +0100
> > Bernhard Reutner-Fischer <rep.dot@gmail.com> wrote:  
> > > On February 13, 2016 12:04:40 PM GMT+01:00, bugzi...@busybox.net wrote:  
> > > >https://bugs.busybox.net/show_bug.cgi?id=6182
> > > >
> > > >Mike Frysinger <vap...@gentoo.org> changed:
> > > >
> > > >   What|Removed |Added
> > > >
> > > >Summary|Add /sbin/nologin   |nologin: new applet
> > > 
> > > We already have that:
> > > https://git.busybox.net/busybox/tree/applets_sh/nologin  
> > 
> > Alpine Linux has a problem with this approach.
> > 
> > On Alpine Linux you have busybox tools by default, but for users who
> > may need the bloaty GNU variants (or any other implementation) they can
> > simply `apk add coreutils` or similar. This works because no package
> > "owns" the applet symlinks pointing to busybox. Instead `busybox
> > --install -s` is run from a trigger[1], whenever any package is
> > installed or removed that has files in /usr/bin /bin etc. This means
> > that when you `apk del coreutils`, the busybox symlinks are restored
> > automatically.
> > 
> > With the shell script "applets" we have the problem that we get real
> > files, "owned" by a package. This causes a conflict error when
> > installing the package. Yes, the package manager can be told to accept
> > overwrites of given files, however, you can not make it automagically
> > restore the busybox applet when you remove the bloated version of it.  
> 
> i'm not against adding a dedicated applet here, but your use case doesn't
> sound like a compelling reason.  sounds more like bad package management.

Do you have a better suggestion on how package manager should solve it?
update-alternatives?

-nc


pgpgLIhRkIaGl.pgp
Description: OpenPGP digital signature
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

adduser/addgroup vs portable useradd/groupadd

[Natanael Copa]

Hi,

As you may know, busybox has been the core of Alpine Linux for a
decade. One of the things that has been necessary for its success has
been that busybox has followed the standards, making the tools
compatible with the corresponding bigger tools.

Even GNU extensions and long-opts have been implemented whenever that
has made sense, and busybox has always been good at doing so in a
portable way.

One exception of this has been the adduser/addgroup tools, and it is
starting to become a problem[1]. Scripts that work on many other
systems needs to be handled special when busybox adduser/addgroup is
needed.

A co-worker did some investigation of the adduser/addgroup vs
useradd/group add implementations on various systems to get an idea
what is the most portable.

I think that what would make most sense is to add useradd and groupadd
to busybox.

What do you think?



useradd, groupadd

Debian, Ubuntu

"useradd is a low level utility for adding users. On Debian,
administrators should usually use adduser(8) instead."

http://manpages.ubuntu.com/manpages/trusty/man8/useradd.8.html
http://manpages.ubuntu.com/manpages/trusty/man8/groupadd.8.html

useradd is not interactive, and supports both long and short command
options, eg -g and --gid. SUpports getting and setting defaults options
with -D. The short options are very similar to many other systems, and
the exit codes correspond to HPUX wg 4=uid alread in use and -o not
specified.

Redhat, Centos

useradd and groupadd are symlinks to adduser, addgroup. These are
basically the same as the Debian, Ubuntu useradd commands, with long
and short form options and the multiple return values for scripting.

Busybox

no useradd or groupadd applets.

HPUX

useradd very like Linux one with only short options eg -g, and supports
-D to set defaults. groupadd only supports -g and -o. Provides 10 exit
codes for different types of failure.

http://ods.com.ua/win/eng/unix/usail/man/hpux/useradd.1.html
http://ods.com.ua/win/eng/unix/usail/man/hpux/groupadd.1.html

claims: STANDARDS COMPLIANCE: useradd: SVID3 groupadd: SVID3

FreeBSD, NetBSD

short form options only, supports -D to change defaults. Man page does
not mention return values.

SVID

System V Interface Definition, Fourth Edition Volume 2
http://www.sco.com/developers/devspecs/vol2.pdf

useradd is specified in here, with short form options, without -D to set 
defaults.
useradd [-u uid [-o] [-i]] [-g group] [-G group[[,group] . . . ]] [-d dir]
[-s shell] [-c comment] [-m [-k skel_dir] -f inactive] [-e expire]
[-h level [-h level [ . . . ]]] [-v def_level] [-w hd_level] [-a event[, . . . 
]]
login

adduser

Debian, Ubuntu

adduser, addgroup are the same perl script. Only accept long form
options.

Redhat, Centos

Exactly same as Debian and Ubuntu version of useradd.

Busybox

Short form options only. Do not correspond to standard useradd options
though - eg -D is used for different purpose.

NetBSD, FreeBSD

No adduser.

--

[1]: 
https://github.com/docker/docker/blob/master/pkg/idtools/usergroupadd_linux.go#L11-L16
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [Bug 6182] nologin: new applet

[Natanael Copa]

On Sat, 13 Feb 2016 13:32:10 +0100
Bernhard Reutner-Fischer  wrote:

> On February 13, 2016 12:04:40 PM GMT+01:00, bugzi...@busybox.net wrote:
> >https://bugs.busybox.net/show_bug.cgi?id=6182
> >
> >Mike Frysinger  changed:
> >
> >   What|Removed |Added
> >
> >Summary|Add /sbin/nologin   |nologin: new applet  
> 
> We already have that:
> https://git.busybox.net/busybox/tree/applets_sh/nologin

Alpine Linux has a problem with this approach.

On Alpine Linux you have busybox tools by default, but for users who
may need the bloaty GNU variants (or any other implementation) they can
simply `apk add coreutils` or similar. This works because no package
"owns" the applet symlinks pointing to busybox. Instead `busybox
--install -s` is run from a trigger[1], whenever any package is
installed or removed that has files in /usr/bin /bin etc. This means
that when you `apk del coreutils`, the busybox symlinks are restored
automatically.

With the shell script "applets" we have the problem that we get real
files, "owned" by a package. This causes a conflict error when
installing the package. Yes, the package manager can be told to accept
overwrites of given files, however, you can not make it automagically
restore the busybox applet when you remove the bloated version of it.

As you may understand this feature is pretty important for Alpine Linux
so we ship our own[2] nologin applet for busybox.


[1]: 
http://git.alpinelinux.org/cgit/aports/tree/main/busybox/busybox.trigger#n18
[2]: http://git.alpinelinux.org/cgit/aports/tree/main/busybox/nologin.c

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 3/3] su: FEATURE_SU_NULLOK_SECURE

[Natanael Copa]

On Thu,  5 Nov 2015 16:27:36 +0200
Kaarle Ritvanen  wrote:

> When this feature is enabled, blank passwords are not accepted by su
> unless the user is on a secure TTY defined in /etc/securetty. This
> resembles the default PAM configuration of some Linux distros which
> specify the nullok_secure option for pam_unix.so.

Denys,

Those 3 patches would be the optimal solution for my blank root password
problem.

- It allows me to create containers with blank root password so i can
  log in via console from host.
- It allows containers run services as non-root without intruders being
  able to elevate privileges with su.
- It makes the configuration for the end user very similar to
  traditional GNU linux using PAM, without depending on the extra PAM
  bloat.
- It adds the functionality in harmony how it was solved in busybox
  'login'. Consistency is good.

I'd be very happy if you could apply those patches.

bloatcheck (x86_64):

function old new   delta
check_securetty- 160+160
su_main  581 611 +30
ask_and_check_password_extended  142 147  +5
ask_and_check_password14  19  +5
login_main  14311299-132
--
(add/remove: 2/0 grow/shrink: 3/1 up/down: 200/-132)   Total: 68 bytes
   textdata bss dec hex filename
 12723536912800  133726   20a5e busybox_old
 12730336912800  133794
 20aa2  busybox_unstripped


-nc

> ---
>  loginutils/su.c | 18 +-
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/loginutils/su.c b/loginutils/su.c
> index 3c0e8c1..85d8e11 100644
> --- a/loginutils/su.c
> +++ b/loginutils/su.c
> @@ -24,6 +24,11 @@
>  //config:bool "Enable su to check user's shell to be listed in 
> /etc/shells"
>  //config:depends on SU
>  //config:default y
> +//config:
> +//config:config FEATURE_SU_NULLOK_SECURE
> +//config:  bool "Disallow blank passwords from TTYs other than specified 
> in /etc/securetty"
> +//config:  depends on SU
> +//config:  default n
>  
>  //applet:/* Needs to be run by root or be suid root - needs to change uid 
> and gid: */
>  //applet:IF_SU(APPLET(su, BB_DIR_BIN, BB_SUID_REQUIRE))
> @@ -76,6 +81,7 @@ int su_main(int argc UNUSED_PARAM, char **argv)
>   struct passwd *pw;
>   uid_t cur_uid = getuid();
>   const char *tty;
> + int allow_blank = 1;
>  #if ENABLE_FEATURE_UTMP
>   char user_buf[64];
>  #endif
> @@ -96,6 +102,12 @@ int su_main(int argc UNUSED_PARAM, char **argv)
>   argv++;
>   }
>  
> + tty = xmalloc_ttyname(STDIN_FILENO);
> + if (!tty) tty = "none";
> + tty = skip_dev_pfx(tty);
> +
> + if (ENABLE_FEATURE_SU_NULLOK_SECURE) allow_blank = check_securetty(tty);
> +
>   if (ENABLE_FEATURE_SU_SYSLOG) {
>   /* The utmp entry (via getlogin) is probably the best way to
>* identify the user, especially if someone su's from a 
> su-shell.
> @@ -109,16 +121,12 @@ int su_main(int argc UNUSED_PARAM, char **argv)
>   pw = getpwuid(cur_uid);
>   old_user = pw ? xstrdup(pw->pw_name) : "";
>   }
> - tty = xmalloc_ttyname(2);
> - if (!tty) {
> - tty = "none";
> - }
>   openlog(applet_name, 0, LOG_AUTH);
>   }
>  
>   pw = xgetpwnam(opt_username);
>  
> - if (cur_uid == 0 || ask_and_check_password(pw) > 0) {
> + if (cur_uid == 0 || ask_and_check_password_extended(pw, 0, allow_blank, 
> "Password: ") > 0) {
>   if (ENABLE_FEATURE_SU_SYSLOG)
>   syslog(LOG_NOTICE, "%c %s %s:%s",
>   '+', tty, old_user, opt_username);

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 3/3] ash: simplify EOF/newline handling in list parser

[Natanael Copa]

On Thu, 29 Oct 2015 11:30:55 +
Ron Yorston <r...@pobox.com> wrote:

> Processing of here documents in ash has had a couple of breakages
> which are now the subject of tests.  This commit should fix both.
> 
> It is based on the following commit in dash git by Herbert Xu:
> 
><7c245aa> [PARSER] Simplify EOF/newline handling in list parser
> 
> (See git://git.kernel.org/pub/scm/utils/dash/dash.git)
> 
> Reported-by: Natanael Copa <nc...@alpinelinux.org>
> Signed-off-by: Ron Yorston <r...@pobox.com>
> ---
>  shell/ash.c | 63
> - 1 file
> changed, 29 insertions(+), 34 deletions(-)
> 
> diff --git a/shell/ash.c b/shell/ash.c
> index 86d648a..9d0226c 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -10516,7 +10516,7 @@ static union node *andor(void);
>  static union node *pipeline(void);
>  static union node *parse_command(void);
>  static void parseheredoc(void);
> -static char peektoken(void);
> +static int peektoken(void);
>  static int readtoken(void);
>  
>  static union node *
> @@ -10525,11 +10525,27 @@ list(int nlflag)
>   union node *n1, *n2, *n3;
>   int tok;
>  
> - checkkwd = CHKNL | CHKKWD | CHKALIAS;
> - if (nlflag == 2 && peektoken())
> - return NULL;
>   n1 = NULL;
>   for (;;) {
> + switch (peektoken()) {
> + case TNL:
> + if (!(nlflag & 1))
> + break;
> + parseheredoc();
> + return n1;
> +
> + case TEOF:
> + if (!n1 && (nlflag & 1))
> + n1 = NODE_EOF;
> + parseheredoc();
> + return n1;
> + }
> +
> + checkkwd = CHKNL | CHKKWD | CHKALIAS;
> + if (nlflag == 2 && tokname_array[peektoken()][0])

probably want nexttoken_ends_list() for this. (which i think is use
more places?)



> + return n1;
> + nlflag |= 2;
> +
>   n2 = andor();
>   tok = readtoken();
>   if (tok == TBACKGND) {
> @@ -10555,30 +10571,15 @@ list(int nlflag)
>   n1 = n3;
>   }
>   switch (tok) {
> + case TNL:
> + case TEOF:
> + tokpushback = 1;
> + /* fall through */
>   case TBACKGND:
>   case TSEMI:
> - tok = readtoken();
> - /* fall through */
> - case TNL:
> - if (tok == TNL) {
> - parseheredoc();
> - if (nlflag == 1)
> - return n1;
> - } else {
> - tokpushback = 1;
> - }
> - checkkwd = CHKNL | CHKKWD | CHKALIAS;
> - if (peektoken())
> - return n1;
>   break;
> - case TEOF:
> - if (heredoclist)
> - parseheredoc();
> - else
> - pungetc();  /* push back
> EOF on input */
> - return n1;
>   default:
> - if (nlflag == 1)
> + if ((nlflag & 1))
>   raise_error_unexpected_syntax(-1);
>   tokpushback = 1;
>   return n1;
> @@ -11947,14 +11948,14 @@ readtoken(void)
>   return t;
>  }
>  
> -static char
> +static int
>  peektoken(void)
>  {
>   int t;
>  
>   t = readtoken();
>   tokpushback = 1;
> - return tokname_array[t][0];
> + return t;
>  }
>  
>  /*
> @@ -11964,18 +11965,12 @@ peektoken(void)
>  static union node *
>  parsecmd(int interact)
>  {
> - int t;
> -
>   tokpushback = 0;
> + checkkwd = 0;
> + heredoclist = 0;
>   doprompt = interact;
>   setprompt_if(doprompt, doprompt);
>   needprompt = 0;
> - t = readtoken();
> - if (t == TEOF)
> - return NODE_EOF;
> - if (t == TNL)
> - return NULL;
> - tokpushback = 1;
>   return list(1);
>  }
>  

you forgot the last hunk in Herbert Xu's patch.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 2/3] Revert "ash: fix a SEGV case in an invalid heredoc" xxx

[Natanael Copa]

On Thu, 29 Oct 2015 12:29:37 +
Ron Yorston <r...@pobox.com> wrote:

> Natanael Copa wrote:
> >I think this will not work. peektoken and nexttoken_ends_lists are not
> >same thing.
> 
> Originally peektoken in Busybox ash was the same as in dash.  It then
> changed to effectively act as nexttoken_ends_lists but the name was
> retained until Denys's patch.  Reverting that patch changes the name
> but retains the old behaviour.  The following patch changes the behaviour
> to match that of dash.
> 
> >I know you correct it in next commit but i think it may break things
> >for future git bisect.
> 
> It works.  Busybox builds at each commit and the tests pass or fail
> as appropriate.
> 
> Ron

I sent a version without a revert, which also includes the last hunk
from Xu's patch in dash. I also kept the nexttoken_ends_lists which I
think makes code more readable and reintroduced the peektoken.

I am not familiar with ash code so I don't know which is most correct.

I also figured that we can save a few bytes with:

-   tokpushback = 1;
+   tokpushback++;

But I don't know if that can cause surprises due to overflows.

I can send a patch for that once Denys decides how to fix it.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 2/3] Revert "ash: fix a SEGV case in an invalid heredoc" xxx

[Natanael Copa]

On Thu, 29 Oct 2015 11:30:22 +
Ron Yorston <r...@pobox.com> wrote:

> This reverts commit 7e66102f762a7d80715f0c7e5925433256b78cee but
> leaves the test in place as it's still valid.
> 
> Reported-by: Natanael Copa <nc...@alpinelinux.org>
> Signed-off-by: Ron Yorston <r...@pobox.com>
> ---
>  shell/ash.c | 15 ---
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/shell/ash.c b/shell/ash.c
> index 17121aa..86d648a 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -10516,7 +10516,7 @@ static union node *andor(void);
>  static union node *pipeline(void);
>  static union node *parse_command(void);
>  static void parseheredoc(void);
> -static char nexttoken_ends_list(void);
> +static char peektoken(void);
>  static int readtoken(void);
>  
>  static union node *
> @@ -10526,7 +10526,7 @@ list(int nlflag)
>   int tok;
>  
>   checkkwd = CHKNL | CHKKWD | CHKALIAS;
> - if (nlflag == 2 && nexttoken_ends_list())
> + if (nlflag == 2 && peektoken())
>   return NULL;
>   n1 = NULL;
>   for (;;) {
> @@ -10568,15 +10568,8 @@ list(int nlflag)
>   tokpushback = 1;
>   }
>   checkkwd = CHKNL | CHKKWD | CHKALIAS;
> - if (nexttoken_ends_list()) {
> - /* Testcase: "<<EOF; then <W".
> -  * It used to segfault w/o this check:
> -  */
> - if (heredoclist) {
> - raise_error_unexpected_syntax(-1);
> - }
> + if (peektoken())
>   return n1;
> - }
>   break;
>   case TEOF:
>   if (heredoclist)
> @@ -11955,7 +11948,7 @@ readtoken(void)
>  }
>  
>  static char
> -nexttoken_ends_list(void)
> +peektoken(void)

I think this will not work. peektoken and nexttoken_ends_lists are not
same thing.

I know you correct it in next commit but i think it may break things
for future git bisect.

>  {
>   int t;
>  

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1.24.1] ash: backport fix for here document issues

[Natanael Copa]

On Thu, 29 Oct 2015 11:31:39 +
Ron Yorston <r...@pobox.com> wrote:

> Reported-by: Natanael Copa <nc...@alpinelinux.org>
> Signed-off-by: Ron Yorston <r...@pobox.com>
> ---
>  shell/ash.c   | 72 
> +--
>  shell/ash_test/ash-heredoc/heredoc2.right |  2 +
>  shell/ash_test/ash-heredoc/heredoc2.tests |  7 +++
>  3 files changed, 39 insertions(+), 42 deletions(-)
>  create mode 100644 shell/ash_test/ash-heredoc/heredoc2.right
>  create mode 100644 shell/ash_test/ash-heredoc/heredoc2.tests

We use this for Alpine Linux til something shows up in
http://busybox.net/downloads/fixes-1.24.1/

Big thanks!

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1.24.1] ash: backport fix for here document issues

[Natanael Copa]

On Thu, 29 Oct 2015 11:31:39 +
Ron Yorston <r...@pobox.com> wrote:

> Reported-by: Natanael Copa <nc...@alpinelinux.org>
> Signed-off-by: Ron Yorston <r...@pobox.com>
> ---
>  shell/ash.c   | 72 
> +--
>  shell/ash_test/ash-heredoc/heredoc2.right |  2 +
>  shell/ash_test/ash-heredoc/heredoc2.tests |  7 +++
>  3 files changed, 39 insertions(+), 42 deletions(-)
>  create mode 100644 shell/ash_test/ash-heredoc/heredoc2.right
>  create mode 100644 shell/ash_test/ash-heredoc/heredoc2.tests


With this patch applied I get problem with this:

if true; then
cat >>confdefs.h <<_ACEOF
#define `date`
_ACEOF
fi


Seems like `command` inside heredoc inside if .. fi is needed to trigger it.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 5/5 v2] ash: fix error during recursive processing of here document

[Natanael Copa]

On Thu, 29 Oct 2015 16:44:56 +
Ron Yorston  wrote:

> Save the value of the checkkwd flag to prevent it being clobbered
> during recursion.
> 
> Based on commit ec2c84d from git://git.kernel.org/pub/scm/utils/dash/dash.git
> by Herbert Xu.

This fixes the issue introduced with the "[PATCH 1.24.1] ash: backport
fix for here document issues" fix.

Denys both, this patch and the "[PATCH 1.24.1] ash: backport fix for
here document issues" are needed for 1_24_stable.

Thanks!

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1.24.1] ash: backport fix for here document issues

[Natanael Copa]

On Thu, 29 Oct 2015 16:22:44 +
Ron Yorston <r...@pobox.com> wrote:

> Natanael Copa wrote:
> >if true; then
> >cat >>confdefs.h <<_ACEOF
> >#define `date`
> >_ACEOF
> >fi
> >
> >Seems like `command` inside heredoc inside if .. fi is needed to trigger it.
> 
> Using $(command) seems to be a workaround.

I cannot patch every autotools configure script :-/

issue is introduced by Xu's simplification patch.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

busybox 1.24.1 ash regression

[Natanael Copa]

Hi,

musl's configure script fails with busybox 1.24.1 ash:

./configure: line 50: syntax error: unexpected "}"

The relevant code from the script

quote () {
tr '\n' ' ' &1 && { echo 
"$1" ; return 0 ; }
$1
EOF
printf %s\\n "$1" | sed -e "s/'/'''/g" -e "1s/^/'/" -e "\$s/\$/'/" -e 
"s#^'\([-[:alnum:]_,./:]*\)=\(.*\)\$#\1='\2#"
}

It is the "tr ... 

Re: busybox 1.24.1 ash regression

[Natanael Copa]

On Thu, 29 Oct 2015 08:16:30 +0100
Natanael Copa <nc...@alpinelinux.org> wrote:

> Simplfied test case:
> 
> 
> foo () {
> tr '\n' ' ' < $1
> EOF
> }
> 
> foo "bar"
> 
> 
> The simplified testcase works with bash, dash and busybox 1.23 ash, but
> fails with busybox 1.24.1.

I bisected it to:

7e66102f762a7d80715f0c7e5925433256b78cee is the first bad commit
commit 7e66102f762a7d80715f0c7e5925433256b78cee
Author: Denys Vlasenko <vda.li...@googlemail.com>
Date:   Thu Feb 5 21:00:17 2015 +0100

ash: fix a SEGV case in an invalid heredoc

Signed-off-by: Denys Vlasenko <vda.li...@googlemail.com>

:04 04 971ee626cc195afe0030c037c376fe895fa683c1
67f020615d2eb44eff814e3bf182df07913d8ab4 M  shell


-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: busybox 1.24.1 ash regression

[Natanael Copa]

On Thu, 29 Oct 2015 09:21:06 +0100
Natanael Copa <nc...@alpinelinux.org> wrote:

> On Thu, 29 Oct 2015 08:16:30 +0100
> Natanael Copa <nc...@alpinelinux.org> wrote:
> 
> > Simplfied test case:
> > 
> > 
> > foo () {
> > tr '\n' ' ' < > $1
> > EOF
> > }
> > 
> > foo "bar"
> > 
> > 
> > The simplified testcase works with bash, dash and busybox 1.23 ash, but
> > fails with busybox 1.24.1.
> 
> I bisected it to:
> 
> 7e66102f762a7d80715f0c7e5925433256b78cee is the first bad commit
> commit 7e66102f762a7d80715f0c7e5925433256b78cee
> Author: Denys Vlasenko <vda.li...@googlemail.com>
> Date:   Thu Feb 5 21:00:17 2015 +0100
> 
> ash: fix a SEGV case in an invalid heredoc
> 
> Signed-off-by: Denys Vlasenko <vda.li...@googlemail.com>
> 
> :04 04 971ee626cc195afe0030c037c376fe895fa683c1
> 67f020615d2eb44eff814e3bf182df07913d8ab4 Mshell

This appears relevant config:

CONFIG_UNICODE_SUPPORT=y

Disabling the CONFIG_UNICODE_SUPPORT makes the problem disappear.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: busybox 1.24.1 ash regression

[Natanael Copa]

On Thu, 29 Oct 2015 09:28:20 +0100
Natanael Copa <nc...@alpinelinux.org> wrote:

> On Thu, 29 Oct 2015 09:21:06 +0100
> Natanael Copa <nc...@alpinelinux.org> wrote:
> 
> > On Thu, 29 Oct 2015 08:16:30 +0100
> > Natanael Copa <nc...@alpinelinux.org> wrote:
> > 
> > > Simplfied test case:
> > > 
> > > 
> > > foo () {
> > > tr '\n' ' ' < > > $1
> > > EOF
> > > }
> > > 
> > > foo "bar"
> > > 
> > > 
> > > The simplified testcase works with bash, dash and busybox 1.23 ash, but
> > > fails with busybox 1.24.1.
> > 
> > I bisected it to:
> > 
> > 7e66102f762a7d80715f0c7e5925433256b78cee is the first bad commit
> > commit 7e66102f762a7d80715f0c7e5925433256b78cee
> > Author: Denys Vlasenko <vda.li...@googlemail.com>
> > Date:   Thu Feb 5 21:00:17 2015 +0100
> > 
> > ash: fix a SEGV case in an invalid heredoc
> > 
> > Signed-off-by: Denys Vlasenko <vda.li...@googlemail.com>
> > 
> > :04 04 971ee626cc195afe0030c037c376fe895fa683c1
> > 67f020615d2eb44eff814e3bf182df07913d8ab4 M  shell
> 
> This appears relevant config:
> 
> CONFIG_UNICODE_SUPPORT=y
> 
> Disabling the CONFIG_UNICODE_SUPPORT makes the problem disappear.

Correction. CONFIG_UNICODE_SUPPORT is irrelevant in this case. I built
tested wrong commit. Sorry.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

request for another missed ifupdown patch

[Natanael Copa]

Hi,

This was sent 2012 but probably forgotten too:
http://lists.busybox.net/pipermail/busybox/2012-September/078398.html

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

request for missed ifupdown ipv6 patch

[Natanael Copa]

This patch seems to have been forgotten and is needed for ipv6 to work properly:
http://lists.busybox.net/pipermail/busybox/2012-August/078297.html

Could it please be added thanks?

I am getting tired of rebasing it every release.

-nc

PS. have you considered using patchwork? It helped alpine linux alot
when patches started to fall between the cracks. Might be
http://patchwork.ozlabs.org/ can host it for you too.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] su: support denying accounts with blank password

[Natanael Copa]

On Tue, 13 Oct 2015 18:03:10 +0200
Denys Vlasenko <vda.li...@googlemail.com> wrote:

> On Mon, Oct 12, 2015 at 12:07 PM, Natanael Copa <nc...@alpinelinux.org> wrote:
> >> I tested
> >>
> >> $ su --version
> >> su (GNU coreutils) 8.17
> >>
> >> and it allows su to root w/o asking for password if it is null.
> >>
> >> busybox does the same.
> >> If there is the need to disallow people to be able to log in as root,
> >> you should set root password.
> >>
> >> If you set password hash to an invalid hash, you can even make people
> >> to be unable to ever login as root - there is no valid password then
> >> (passwd -l does this).
> >
> > And what if you need remote root users to log in with ssh key for
> > remote administration but in emergency situations you need a local
> > technician to log in locally?
> 
> I don't understand the scenario. You propose to set root password to
> empty string ("root::...") and disallow su to it.

The scenario is that you ship a vpn box/firewall to protect the "local
technician". Under normal circumstances the local technician never logs
in or never touch the busybox powered network device. All
administration happens remotely via ssh, using ssh keys.
 
> The "local technician" in this scenario is supposed to log in how?

using a screen and keyboard or via serial cable. he logs in as root,
but is not asked for password or just press  when asked for
password.

> Why is this good in general? With /bin/login, root with empty
> password will not ask for password. This doesn't look more secure
> than well-known password, it's even less secure.

The security is based on physical access. The local technician can log
in without password. (in theory, if you have physical access then you
have access to it all anyway). And after all, it is the "local technician"
the device is supposed to protect anyway.

The only time the local technician may need to log in is if there is an
emergency. A situation where there is full panic and the local
technician (which is not technical at all likely) will try fix the
situation with help from a real technician on the phone.

> Please give more details.

The idea is that if you have physical access, then you can get full
access anyway, regardless there is password or not.

You can currently set up a box (or lxcontainer) without password. It
works great, except that if you log in as normal user (remotely or
local) you can simply do 'su' to get full root access. This means that
if a non-root service gets compromised (remote exploit), the attacker
can simply do 'su' and get full root. That means that the privilege
separation layer is useless and you end up with a situation where all
services just as well could have run as root.

The simple way to fix that, is to configure su to not work unless root
has a password.

Ideally, su should also consult /etc/securetty so it becomes a runtime
configuration instead of compile time. I can prepare a patch for that
if you want.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] su: support denying accounts with blank password

[Natanael Copa]

On Wed, 14 Oct 2015 10:45:49 +0200
Denys Vlasenko  wrote:

> Basically, you want root to have no password and yet,
> you want people to not be able to su to root.

Correct. The thinking is that empty password is better than a bad
password.

> I find this setup strange. You deliberately remove the thing
> which was _designed_ to prevent people to become root
> (the password), and then you add hacks ("su won't accept
> empty password") to plug the hole you just created.
> 
> Do you plan to also teach all other utilities to ignore
> empty password? ssh, login, ftpd...

yes. ftpd does not run as suid root, right? then it is no problem.

sshd has config options for disallow root at all, disallow password
logins (and use ssh keys only) or disallow blank passwords.

> For example, login (at least busybox's one) *works from
> non-root shell*. You can run it from shell, yes.
> It will ask for username, and if user has no password,
> it will log you in!

There is a config option to make busybox login consult securetty.

The only thing left to teach to ignore empty password is busybox su.
Thus this patch.
 
> I continue to think that if you don't want people
> to log in as root, you should simply set root password.

This feature would be for people who want people to log in as root
locally, while still have privilege separation for network services.

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] su: support denying accounts with blank password

[Natanael Copa]

On Wed, 14 Oct 2015 05:43:32 -0400
Michael Conrad <mcon...@intellitree.com> wrote:

> On 10/14/2015 2:37 AM, Natanael Copa wrote:
> > The security is based on physical access. The local technician can log
> > in without password. (in theory, if you have physical access then you
> > have access to it all anyway). And after all, it is the "local technician"
> > the device is supposed to protect anyway.
> 
> Why run 'login' at all?  You can just run "agetty -l /bin/bash" from 
> init or runit and always have a shell ready.

This is a valid alternative approach that has been discussed.

The drawback is that if you actually want a root password you will have
to edit /etc/inittab while with 'su' you'll only need set password.

This actually matters for me because i want use no password as default
for the alpine linux LXC template. Then you can just create the
container and set password. If you forget the set password step you are
still more or less safe. (sshd default config disallows root login with
empty password).

> Alternatively you can randomize the password and print it on the
> console with the welcome message.

This is an interesting idea.
 
> Just some other ideas.
> -Mike
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] su: support denying accounts with blank password

[Natanael Copa]

On Wed, 14 Oct 2015 12:18:04 +0200
Laurent Bercot <ska-dietl...@skarnet.org> wrote:

> On 14/10/2015 08:37, Natanael Copa wrote:
> > using a screen and keyboard or via serial cable. he logs in as root,
> > but is not asked for password or just press  when asked for
> > password.
> 
>   What companies usually do in this case (typically ISPs with modems
> they ship to users) is set a trivial root password, such as "admin",
> and disable privilege-gaining binaries entirely, except /bin/login
> which checks /etc/securetty.
>   It's not much harder for a non-technical user to log in with a
> trivial password than with no password at all, and it ensures that
> only local users can log in as root. (Of course, ISPs have their
> own backdoors into those modems, but that is another story.)

This is also what many distros do in their LXC templates. Problem is
that users start sshd and sshd does not disable root login with
password by default. So container provider start container and before
user reach to log in to set password they are compromised.

see https://github.com/lxc/lxc/issues/302

-nc
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox