RE: [PATCH] utils: Remove always true checks

[David Laight]

On 64bit long is 64bits - so the test is valid.

> -Original Message-
> From: busybox  On Behalf Of Maks Mishin
> Sent: 22 August 2024 18:04
> To: busybox@busybox.net
> Subject: [PATCH] utils: Remove always true checks
> 
> Expression 'res <= 4294967295UL' is always true , which may be caused
> by a logical error: 'res' has a type 'unsigned long' with minimum value '0'
> and a maximum value '4294967295'
> 
> Found by the static analyzer Svace.
> 
> Signed-off-by: Maks Mishin 
> ---
>  networking/libiproute/utils.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/networking/libiproute/utils.c b/networking/libiproute/utils.c
> index 3cce4a06e..9e8600f34 100644
> --- a/networking/libiproute/utils.c
> +++ b/networking/libiproute/utils.c
> @@ -42,7 +42,7 @@ unsigned FAST_FUNC get_unsigned(char *arg, const char 
> *errmsg)
>   if (*arg) {
>   res = strtoul(arg, &ptr, 0);
>  //FIXME: "" will be accepted too, is it correct?!
> - if (!*ptr && res <= UINT_MAX) {
> + if (!*ptr) {
>   return res;
>   }
>   }
> @@ -57,7 +57,7 @@ uint32_t FAST_FUNC get_u32(char *arg, const char *errmsg)
>   if (*arg) {
>   res = strtoul(arg, &ptr, 0);
>  //FIXME: "" will be accepted too, is it correct?!
> - if (!*ptr && res <= 0xUL) {
> + if (!*ptr) {
>   return res;
>   }
>   }
> --
> 2.30.2
> 
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: AW: Re busybox tar hidden filename exploit

[David Laight]

From: Michael Conrad
> Sent: 03 July 2024 01:29
> 
> The underlying root problem here is the same as SQL injection or HTML
> cross-site scripting attacks.  You have data, and you emit it in a
> context that is expecting a language/protocol of some sort, not raw
> data.  You then need to escape anything in your data that could be
> misinterpreted as the protocol.  We're really lucky that there isn't any
> way to make a TTY execute commands or delete files or grant user
> permissions.

I'm sure some terminals supported an escape sequences to write the
terminal 'answerback' message.
(You might need to back to 1980's async terminals.)

Having 'ls' generate the answerback message (unlikely on anything recent)
is mighty confusing - even when not malicious.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH] vi: add +LINE support

[David Laight]

From: Fredrik Flornes Ellertsen
> Sent: 04 June 2024 09:24
> To: busybox@busybox.net
> Subject: [PATCH] vi: add +LINE support
> 
> Since v253 (commit 1ae886f), systemd assumes that $EDITOR understands the 
> +line
> syntax to jump to a specific line when opening a file with 'systemctl edit'
> (vim, ed, and nano all do). systemd's assumption of this won't go away [0], so
> BusyBox vi might as well support it.
> 
> [0] https://github.com/systemd/systemd/issues/33023#issuecomment-2145506204
> 
> Signed-off-by: Fredrik Flornes Ellertsen 
> ---
>  editors/vi.c | 23 +--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/editors/vi.c b/editors/vi.c
> index 3cc3d2a0b..964ca0132 100644
> --- a/editors/vi.c
> +++ b/editors/vi.c
> @@ -181,11 +181,12 @@
>  //kbuild:lib-$(CONFIG_VI) += vi.o
> 
>  //usage:#define vi_trivial_usage
> -//usage:   IF_FEATURE_VI_COLON("[-c CMD] ")IF_FEATURE_VI_READONLY("[-R] 
> ")"[-H] [FILE]..."
> +//usage:   IF_FEATURE_VI_COLON("[-c CMD] ")IF_FEATURE_VI_READONLY("[-R] 
> ")"[-H] [+line]
> [FILE]..."
>  //usage:#define vi_full_usage "\n\n"
>  //usage:   "Edit FILE\n"
>  //usage: IF_FEATURE_VI_COLON(
>  //usage: "\n -c CMD  Initial command to run ($EXINIT and ~/.exrc 
> also available)"
> +//usage: "\n +LINE   Open FILE with cursor at LINE"
>  //usage: )
>  //usage: IF_FEATURE_VI_READONLY(
>  //usage: "\n -R  Read-only"
> @@ -349,6 +350,7 @@ struct globals {
>   int get_rowcol_error;
>  #endif
>   int crow, ccol;  // cursor is on Crow x Ccol
> + int open_at_line;// open file at line (default 0)
>   int offset;  // chars scrolled off the screen to the left
>   int have_status_msg; // is default edit status needed?
>// [don't make smallint!]
> @@ -483,6 +485,7 @@ struct globals {
>  #define columns (G.columns)
>  #define crow(G.crow   )
>  #define ccol(G.ccol   )
> +#define open_at_line(G.open_at_line   )
>  #define offset  (G.offset )
>  #define status_buffer   (G.status_buffer  )
>  #define have_status_msg (G.have_status_msg)
> @@ -4871,6 +4874,8 @@ static void edit_file(char *fn)
>   while (initial_cmds)
>   run_cmds((char *)llist_pop(&initial_cmds));
>  #endif
> + dot = find_line(open_at_line);
> + dot_skip_over_ws();
>   redraw(FALSE);  // dont force every col re-draw
>   //--This is the main Vi cmd handling loop ---
>   while (editing > 0) {
> @@ -5034,10 +5039,24 @@ int vi_main(int argc, char **argv)
>   }
>   }
>  #endif
> +
> + // 4: process optional +line argument
> + optind = 0;
> + open_at_line = 0;
> + if (argv[optind]) {
> + int pos;
> + if (sscanf(argv[optind], "+%d%n", &open_at_line, &pos) == 1) {
> + if (strlen(argv[optind]) == pos) {

if (!argv[optind][pos])

> + optind++;
> + } else {
> +  // garbage in +line argument, assume it's a 
> filename
> + open_at_line = 0;
> + }
> + }
> + }
>   // "Save cursor, use alternate screen buffer, clear screen"
>   write1(ESC"[?1049h");
>   // This is the main file handling loop
> - optind = 0;
>   while (1) {
>   edit_file(argv[optind]); // might be NULL on 1st iteration
>   // NB: optind can be changed by ":next" and ":rewind" commands
> --
> 2.45.0
> 
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: Off-topic: filtering stream of pathnames for glob'd exclusions

[David Laight]

From: Philip Prindeville
> Sent: 22 April 2024 17:16
> Somewhat off-topic post but this is on a system that has Busybox installed so 
> I can't use features
> that other shells might provide.
> 
> I have a stream generated by "find" of pathnames, and I want to delete 
> (filter out) certain paths
> based on the contents of a file that holds exclusions.
> 
> Anyone know an easy way to do this?  Something like "... | grep -v -f 
> exclusions.txt" if grep worked
> with globs instead of patterns...

A shell 'case' statement will do filename 'glob' matching.
so something like:
find ... | while read path; do
case $path in
(exclusion_1 | exclusion_2 ...) ;;
(*) echo $path;;
esac
done
might work (modulo some extra quoting - probably ' around the patterns).
That does require a pre-process of your exclusions.txt file.
With a bit of care you can use the shells eval on the entire case statement
to do that in the script itself.
If you use eval make sure you quote everything except the variables you
want expanded on the first pass.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: Print full username and groupname for ls command

[David Laight]

From: Denys Vlasenko
> Sent: 25 February 2024 00:06
> 
> I think we can simply stop the truncation, like this:
> 
> -   column += printf("%-8.8s %-8.8s ",
> +   column += printf("%-8s %-8s ",
> 
> without additional loop to measure max length.

That code isn't safe with a standard libc.
If printf() actually does a write() for any reason and the write
fails then printf() returns -1 not the length.

If you want to check for error (eg disk full) then checking the
return value of printf() is also fairly pointless.
You really need to call fflush() and then ferror().

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: Failing shell code under busybox 1.36.1 that worked with 1.31.1

[David Laight]

From: David Leonard
> Sent: 15 February 2024 08:46
> To: Harvey 
> Cc: busybox@busybox.net
> Subject: Re: Failing shell code under busybox 1.36.1 that worked with 1.31.1
> 
> 
> > _result="$_result '${_p//'/'\"'\"'}'"
> 
> Busybox ash has a CONFIG_ASH_BASH_COMPAT config to get this ${var//...}
> replacement behaviour.  And, in bash, the ' in the pattern section is treated
> as a quote start. To give the ' a literal meaning, use \'
> eg ${_p//\'/..repl..}
...

It is probably safer to the quotes into variables.
Then the syntax analysis won't get messed up.

So something like:
sq="'"
qsq='"'"'"'"'

then
"${_p/$sg/$qsq}"

is probably ok and maybe more readable.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: Failing shell code under busybox 1.36.1 that worked with 1.31.1

[David Laight]

From: Harvey
> Sent: 14 February 2024 18:18
> Sorry, posted the wrong version of the pack_args() funtion:
> I have payed with possible solutions for so long that I forgot the
> restore the original before posting  -blush-
> 
> Here is the original version:
> 
> 
> # Packs arguments into a single string.
> #
> # Input:
> #   $1 = name of variable receiving the arguments in such a way that
> #(re)evaluating them provides the original arguments
> #   $2... = arguments
> # Example:
> #   func() {
> # local args
> # pack_args args "$@"
> # send_rpc func_impl "$args"
> #   }
> #   # (in another process)
> #   receive_rpc() {
> # local func="$1"
> # local args="$2"
> # eval $func "$args"
> #   }
> pack_args()
> {
>  local _p _resvar=$1 _result= _value
>  shift
>  for _p
>  do
>  _result="$_result '${_p//'/'\"'\"'}'"
>  done
>  eval $_resvar=\${_result\# }
> }

Try just splitting the quoting of the final '
Just adding "" before it might be enough.
Failing that add it on the next line.

Probably easier than trying to find the bug in ash.
(been there, done that...)

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH] crond: add LOG_NDELAY to openlog

[David Laight]

From: Jones Syue
> Sent: 12 December 2023 01:49
> 
> > You aren't allowed to do very much between vfork() and exec().
> > Basically just sort out the child's fd table.
> > And I suspect that needs to only look at on-stack data.
> >
> > Having the child execute first is only an optimisation.
> > The child can fault on paged-out memory (even its stack) which
> > would allow the parent to run.
> 
> Yes. Definitely!  :)
> And per man page 'On some implementations, vfork() is equivalent to fork().'
> race conditions between parent and children is possible too,
> depends on implementation.
> https://linux.die.net/man/3/vfork
> https://man7.org/linux/man-pages/man2/vfork.2.html
> 
> A possible approach is: let crond parent prepare the DGRAM socket fd
> connecting to /dev/log, avoid been delayed connecting until the first
> message is logged by vfork() children.

I'd actually suggest using fork() - much safer.
While slightly slower no one is going to notice the difference
when cron is running programs.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] crond: add LOG_NDELAY to openlog

[David Laight]

From: Jones Syue
> Sent: 11 December 2023 07:59
>
> After digging further into textbook APUE 2/e and TLPI in this weekend,
> it looks like more clear to explain what is happening.
> In vfork() implementation,
> 1. The process address space is shared among parent and child processes.
> The process address space, including data, stack, and heap, which means any
> modification to a static variable (in data segment) is visible to others.
> That's why vfork() is much faster and less memory footprint than fork(),
> which is very efficient for entry-level cpu in embedded system.
> 2. The file descriptors table is not shared among parent and child processes.
> Parent and children have its own fd table, because vfork() finally call
> clone() syscall without CLONE_FILES.
> 
> After vfork() and before exec(), just in the middle of vfork() and exec(),
> once the syslog() is launched and this 1st-time-call created the DGRAM socket
> to the /dev/log,

You aren't allowed to do very much between vfork() and exec().
Basically just sort out the child's fd table.
And I suspect that needs to only look at on-stack data.

Having the child execute first is only an optimisation.
The child can fault on paged-out memory (even its stack) which
would allow the parent to run.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: fix large PID overflow their column in top command

[David Laight]

...
> +   fp = xfopen_for_read("/proc/sys/kernel/pid_max");
> +   if (!fgets(pid_buf, PID_DIGITS_MAX + 1, fp)) {
> ...
> +   if (strncmp(pid_buf, "32768", 5) == 0)
> +   pid_digits_num = 5;
> +   else
> +   pid_digits_num = PID_DIGITS_MAX;
> 
> The logic above is not sound. Even if sysctl kernel.pid_max
> is 32768, there can be already running processes with pids > 9.

It's also probably wrong for pretty much all other values.

I'd just base the column width on strlen(pid_buf) with a minimum
value of 5.

It is unlikely that pid_max has been reduced - so column overflow
it that case probably doesn't really matter.

The more interesting case is really a system with a very large pid_max
that has never run many processes.
You don't really want lots of blank space.

I can't remember whether top reads everything before doing any output?
Since the output is sorted it probably almost always does.
In which case it knows the column width it will need.

I did post a patch a while back that enabled 'Irix mode'.
(100% cpu is one cpu at 100%, not all cpus at 100%)
Maybe I should dig it out again.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: RFC: initialize /dev/urandom, is it necessary? Can we do it in a better way?

[David Laight]

> Fine, then use > (write) to reset the entropy and >> (append) to add entropy.

IIRC writing to /dev/urandom doesn't do what you want it to do.
You have to use an ioctl() to actually set entropy.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] udhcpd: Per-client boot_file

[David Laight]

From: busybox  On Behalf Of Michael Tokarev
> Sent: 25 June 2023 06:58
> 
> 25.06.2023 02:23, Adam Goldman wrote:
> > This patch adds the ability to specify a different boot_file for each
> > client. This is useful if clients have different CPU architectures or if
> > some clients are legacy BIOS and some are EFI.
> >
> > It adds a config file option of the form
> > "for xx:xx:xx:xx:xx:xx boot_file foo" which sets the boot_file to "foo"
> > for the client with the MAC address xx:xx:xx:xx:xx:xx. If no such line
> > exists, the global boot_file will be used. The syntax of the option is
> > intended so that in the future, per-client options other than boot_file
> > could be added.
> 
> I don't think this scales. Instead of per-client, it needs to be per
> group of clients based on some criteria (such as CPU architecture).
> See for example how dnsmasq does this (and maybe it is better suited
> for your usage).

Isn't there are historic precedence for this to do with booting
diskless clients (eg NetBSD) where there are also options for
supplying different first and second level 'boot' files.

A separate (but maybe interesting) case it to use different
address pools for different MAC OUI.
In particular this would allow the crappy Cisco phones that
never renew addresses be given addresses in a different
range from everything else.
(Although since 'dayjob' insist on using the M$ DNS server
we just have to live with the fact that it is completely
broken.)

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] mount: add option to disable NFS completely

[David Laight]

From: Reimu
> Sent: 25 May 2023 15:04
> 
> This can save up to ~40 kbytes with uclibc and it helps a lot when
> you're building a very tiny busybox for embedded systems.
> ---
>  util-linux/mount.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/util-linux/mount.c b/util-linux/mount.c
> index 4e65b6b46..1c3e2bf64 100644
> --- a/util-linux/mount.c
> +++ b/util-linux/mount.c
> @@ -77,6 +77,17 @@
>  //config:    Note that this option links in RPC support from libc,
>  //config:    which is rather large (~10 kbytes on uclibc).
>  //config:
> +//config:config FEATURE_MOUNT_NFS_ALL
> +//config:    bool "Support mounting NFS file systems on Linux >= 2.6.23"
> +//config:    default y
> +//config:    depends on MOUNT
> +//config:    help
> +//config:    Enable mounting of NFS file systems on Linux kernels with
> +//config:    at least version 2.6.23.

I don't think you need to reference the Linux kernel version.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH] nslookup: ensure unique transaction IDs for the DNS queries

[David Laight]

From: Laurent Bercot
> Sent: 10 May 2023 12:36
> 
> >You miss the point. CLOCK_MONOTONIC may simply be too granular
> >on some hardware - returning the same value for the duration of
> >several milliseconds.
> 
>   Wait, what? Is that a thing? Is there actual hardware where
> CLOCK_MONOTONIC stalls for a noticeable period of time?
>   That does not sound permitted by POSIX:

Some implementation will increment the nanoseconds on
every call to avoid returning the same value.

But the underlying timer might only change every 10ms (or more).
Even though the output value has a nanosecond part.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH v2] readlink: slight size optimization

[David Laight]

From: Harald van Dijk
> Sent: 17 April 2023 22:44
...
> I am assuming it was intended to be
> 
>printf("%s%s", buf, &"\n"[opt & 1]);

That is fine, as is "\n" + (opt & 1)
It can also be written:
&(opt & 1)["\n"]
since [] is commutative in C.
Basically x[y] <=> *(x+y) <=> *(y+x) <=> y[x].

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: busybox seq doesn't handle negative numbers

[David Laight]

...
> Yeah, the thing rather is that GNU getopt parses the command line
> and performs reorders.  I never really looked, but since the
> (entire family of the) mailer i maintain earned a security
> advisory for possible option injection attacks i always wondered
> how secure that can be...

Yes, that is entirely broken and should never have been committed.
I have to remember to add the 'magic character' to disable it.

Historically a few programs use nonstandard argument ordering.
Most notably 'rlogin hostname -l username' but that really
doesn't justify how gnu getopt() works.

Programs like tail, seq and sort are old and have argument
parsing that (probably) predates the standard.
(Although 'sort +4' seems to have been disabled even though
it worked fine for over 30 years.)

I wonder if there is an easy way to 'escape' from busybox
getopt's 'unknown option' error path without printing a
message (and them being able to print the message) so that
programs like seq can decide that -12 isn't actually invalid.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: Problem with test -r

[David Laight]

From: Dominique Martinet
> Sent: 13 February 2023 23:22
...
> - Reproduce under strace and provide the full strace.log:
> strace -o /tmp/strace.log bash -c '[ -r file ]; echo $?'
> strace might not want to run within the container without some extra
> privileges, it's also possible to attach your bash process from outside
> if that's easier, but if you can reproduce with the container running
> with --privileged that is probably the most straightforward.

If the strace just shows the stat (maybe fstatat) system call failing
then the next step is to get ftrace to monitor the kernel function
calls to help guess where the error comes from.
That might be tricky to arrange inside the container.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: Problem with test -r

[David Laight]

From: alice
> Sent: 13 February 2023 09:24

> > The 'test/[' command is almost certainly a shell builtin.
> > The program in /usr/bin is almost never executed.
> 
> when one is using busybox ash as a shell, is /usr/bin/test (from busybox
> symlink) and "builtin test" not the same code in the end (same for [)? i.e.
> this case, which is probably using busybox ash (needs confirmation).

On my busbox system [ is a shell builtin and /usr/bin/[ a program.
Both just call stat(filename) for test -r filename.
I don't get a fail for a non-readable directory (but can only
run as root).

Plausibly there is some hackery involved, but putting programs
like 'ash' into busybox probably requires linking them to a .o
and then making most of the symbols static and renaming 'main'.
So it is difficult to share the code for 'test' unless is
is all moved out into a common library.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: Problem with test -r

[David Laight]

From: Vladimír Macek
> Sent: 11 February 2023 19:15
> 
> Hi,
> 
> I'd like to report what seems to be a bug in the busybox's test command. I
> use the official alpine:3 docker container.
> 
> The test -r does not return true even when the file is indeed readable
> (testing by head command). Yes, the file has an unusual set of permissions,
> but that does not restrict it from reading. Here's the output of the
> interactive container shell session:
> 
> dd72078df6d2:/task/output/files$ ls -lad . .. cnb_rate_eur_czk.csv
> drwxrwx---2 setupproject4096 Feb 11 18:33 .
> drwxr-x---5 setupproject4096 Feb 11 18:33 ..
> -rw-r--r--1 task project   20512 Feb 11 18:33 cnb_rate_eur_czk.csv

By the look of it the problem is that 'test -r' requires
directory search access - so why not just say that.

> dd72078df6d2:/task/output/files$ [ -r cnb_rate_eur_czk.csv ] && echo READABLE
> dd72078df6d2:/task/output/files$ [[ -r cnb_rate_eur_czk.csv ]] && echo 
> READABLE
> 
> dd72078df6d2:/task/output/files$ which [
> /usr/bin/[
> dd72078df6d2:/task/output/files$ which [[
> /usr/bin/[[

The output from 'which' doesn't necessarily have any connection
with the code any specific shell executes for a command.
'which' is a bourne shell script that is trying (and must fail) to
emulate a csh builtin command.

The 'test/[' command is almost certainly a shell builtin.
The program in /usr/bin is almost never executed.
As for '[[', IIRC that is a bash 'special' that nothing
portable should be using.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH] build system: fix linking on Red Hat systems

[David Laight]

From: Michael Tokarev
> Sent: 31 December 2022 09:44
> 
> 31.12.2022 11:48, Ron Yorston wrote:
> > Commit 77216c368 (Fix non-Linux builds) made linking with libresolv
> > conditional on the output of 'gcc -dumpmachine' containing 'gnu'.
> >
> > This broke builds on Red Hat systems (Fedora, RHEL) and derivatives
> > (CentOS, Rocky).  Such systems report a machine type of the form
> > 'x86_64-redhat-linux'.
> >
> > Check for 'linux' as well as 'gnu'.
> 
> This is - in my opinion - both wrong test to begin with, and a wrong
> fix. -lresolv is needed when nslookup applet is enabled, because it
> uses res_XXX functions, - this is regardless of the system, I think.
> I submitted a patch about this a few months ago.  If anything, this
> can be made a test whenever we actually have and need -lresolv by
> doing a small compile test.

You don't need to check whether libresolv is needed just whether
it exists.
The linker won't (usually) add the NEEDED entry unless the library
actually defines some undefined symbols.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] ed: don't use memcpy with overlapping memory regions

[David Laight]

From: Sören Tempel
> Sent: 22 December 2022 14:23
> 
> PING.
> 
> Any love for good old ed(1)?

Some versions of glibc will also do 'not strictly increasing'
memcpy() on some x86 cpu.
Just copying the last 4/8 bytes first and then doing a forwards
copy is enough to break things.

David

> 
> Sören Tempel  wrote:
> > Pinging again as this is still unfixed and the proposed fix is rather 
> > trivial.
> >
> > Sören Tempel  wrote:
> > > Ping.
> > >
> > > soe...@soeren-tempel.net wrote:
> > > > From: Sören Tempel 
> > > >
> > > > The memcpy invocations in the subCommand function, modified by this
> > > > commit, previously used memcpy with overlapping memory regions. This is
> > > > undefined behavior. On Alpine Linux, it causes BusyBox ed to crash since
> > > > we compile BusyBox with -D_FORTIFY_SOURCE=2 and our fortify-headers
> > > > implementation catches this source of undefined behavior [0]. The issue
> > > > can only be triggered if the replacement string is the same size or
> > > > shorter than the old string.
> > > >
> > > > Looking at the code, it seems to me that a memmove(3) is what was
> > > > actually intended here, this commit modifies the code accordingly.
> > > >
> > > > [0]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13504
> > > > ---
> > > >  editors/ed.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/editors/ed.c b/editors/ed.c
> > > > index 209ce9942..4a84f7433 100644
> > > > --- a/editors/ed.c
> > > > +++ b/editors/ed.c
> > > > @@ -720,7 +720,7 @@ static void subCommand(const char *cmd, int num1, 
> > > > int num2)
> > > > if (deltaLen <= 0) {
> > > > memcpy(&lp->data[offset], newStr, newLen);
> > > > if (deltaLen) {
> > > > -   memcpy(&lp->data[offset + newLen],
> > > > +   memmove(&lp->data[offset + newLen],
> > > > &lp->data[offset + oldLen],
> > > > lp->len - offset - oldLen);
> > > >
> > > > ___
> > > > 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
> > ___
> > 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

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: udhcpc6 missing source address

[David Laight]

From: Denys Vlasenko
> Sent: 14 December 2022 21:17
...
> For comparison, this is how DHCPv4 looks on the wire:
> 
> 28:df:eb:11:48:92 > Broadcast, ethertype IPv4 (0x0800), length 342:
> (tos 0x0, ttl 64, id 0, offset 0, flags [none], proto UDP (17), length
> 328)
> 0.0.0.0.bootpc > 255.255.255.255.bootps: BOOTP/DHCP, Request from
> 28:df:eb:11:48:92, length 300, xid 0x158e8233, Flags [none]
>   Client-Ethernet-Address 28:df:eb:11:48:92
> ...
> 40:3f:8c:7c:7a:2d > 28:df:eb:11:48:92, ethertype IPv4 (0x0800), length
> 342: (tos 0xc0, ttl 64, id 41793, offset 0, flags [none], proto UDP
> (17), length 328)
> 192.168.1.1.bootps > 192.168.1.193.bootpc: BOOTP/DHCP, Reply,
> length 300, xid 0x158e8233, Flags [none]
>   Your-IP 192.168.1.193
>   Server-IP 192.168.1.1
> 
> Not only the server is unfazed by source IP of 0.0.0.0, it
> nevertheless sends _unicast_ reply,
> because it saw client's MAC address. (It even filled our would-be IP
> in the destination IP field,
> to be extra nice).

The 0.0.0.0 source IP is just a fudge, there is nothing
else that can be put in the field.
And you'll probably find the both the dchp client and server
side are using the raw packet interface (with a BPF filter)
to receive the packets.
A Linux system that runs the dhcp client never reports any
received ethernet packets as 'discarded by software' because
they all get passed to dhcp!
(At least one time I looked anyway)

The IPv6 broadcast UDP packet (from the link-local address)
is much more of a standard UDP packet than anything dhcp4 uses.

Quite why the system in failing to allocate/return it's
link-local address is another matter entirely.
I think there is a standard wrapped that waits for the link-local
address to appear before starting udhcpc6, we use this shell code:

wait_link_local()
{
# Wait for the ipv6 link local address to appear.
# Probably needed for dhcp6 to work at all.
eth=$1
shift
while
ip_addr=$(ip "$@" address show dev "$eth" scope link 
2>/dev/null) || return 1
[ "${ip_addr#*inet6 fe80}" = "$ip_addr" ]
do
sleep 0.5
done

return 0
}

We do have the full 'ip' command, not just the busybox version.

I'm not sure what happens if you try to start udhcpc6 too soon.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: udhcpc6 missing source address

[David Laight]

From: Denys Vlasenko
> Sent: 14 December 2022 10:04
> 
> On Wed, Dec 14, 2022 at 10:52 AM David Laight  wrote:
...
> If it's the code from the above, then no, kernel will not fill in
> any IPv6 source addresses, as it does not even know it's
> an IPv6/UDP packet - we use raw sockets here,
> and we format UDP packet "by hand":

Not enough coffee this morning to quickly interpret the strace output :-(

I'm pretty sure there is a way of sending UDP multicast packets
and that dhcpc6 could use it.
(It is harder for IPv4 because there isn't a valid source IP.)

We've got some code that sends UDP from RAWIP sockets.
That allows the sender to change the source port for
every message.
For IPv6 the kernel can calculate the UDP checksum after
doing the packet routing - so we don't have to build the
IPv6 header.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: udhcpc6 missing source address

[David Laight]

From: John Lemonovich
> Sent: 13 December 2022 20:35
> 
> How are you getting a solicit message through with  ::  as the source 
> address, which I've found no
> server likes as I've tested 3 of them?  Or is mine not using the link local 
> address because I'm not
> doing anything with the default.script?
> 
> The kernel is 5.4.13

I've just run a quick test.

strace of uhdcpc6 shows it doing something with a routing socket,
creating an IPv6 UDP socket, binding (probably to the interface)
and then doing s sendto() that specifies the multicast MAC.
(It puts the mcast address in the bind as well - probably ignored?)

The kernel is providing the source IPv6 address as part of its
normal UDP routing operations - the same as it does for any
other outbound packet.

tcpdump then shows the packet being sent from the link-local
address - as expected.

Three messages are sent, no reply received (I don't have a server
here and there is absolutely nothing else on that network segment).
The script is run 'leasefail' and it all retries a short time later.

On a network with a server it gets a response.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: udhcpc6 missing source address

[David Laight]

From: John Lemonovich
> Sent: 13 December 2022 15:59
> 
> Thanks for the replies.  Yes David, it's also my understanding per the spec, 
> that the client's link-
> local address must be included for the solicit message.  The reply from the 
> server is unicast.  What
> do you mean by the proper IPv6 version of the script...to which script are 
> you referring to?  Can you
> possibly show an example?

The one that probably ends up in /usr/share/udhcpc/default.script
and is passed to udhcpc6 with the -s option.
Not the least of the problems is that you need to explicitly
delete the old IPv6 address (in deconfig).
I think the script I started from deleted the IPv4 address!

There is also the issue that you need to do a 'mark and scan'
pass over the routes (even in IPv4) to ensure the system isn't
left with an invalid routing table during renew processing.

> Just out of curiosity, has it ever been tested or known to work with a DHCPv6 
> server then?

Works for us (in testing anyway).
There has also been a bug to do with release renewal.
Might have been fixed. If any of our customers decide to use IPv6
I'll need to check - they clearly haven't on the previous product!
(The lack of bugs is a pretty good hint.)

I presume you are running a reasonably recent kernel.
Anything really archaic might be buggy - but you should have a few years.

Are you actually running Linux on a nios2?
I can't actually imagine that being fast enough for anything useful.
(Or does the arria10 contain an arm core?)
We do use nios cpu - but only running a few kb of code.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: udhcpc6 missing source address

[David Laight]

From: Denys Vlasenko
> Sent: 13 December 2022 15:22
> 
> On Fri, Dec 9, 2022 at 5:09 PM John Lemonovich
>  wrote:
> > Hello Denys,
> >
> > I am working on an embedded Linux project using an intel Arria 10 FPGA and 
> > have been using IPv4 for
> a long time, and now need IPv6.  I found your email online searching for 
> help.  I have many things
> working with IPv6, but the one thing I can’t get to work is udhcpc6 – it will 
> not acquire an IP
> address via DHCPv6 (tried with multiple servers).  I’m really stuck and 
> looking for any help – even a
> pointer to documentation maybe I’m missing?
> 
> Sorry, I myself have no DHPCv6 server to test against...
> 
> > If I look at the tcpdump output – the solicit packets go out with a null 
> > source address (all 0’s  -
> just as  ::  )
> 
> Which kind of makes sense, if you think about it. DHCP client does not
> _have_ an IP address yet,
> this is the whole point of DHCP: to obtain an address...

They do need to go out from the link-local address.
Whatever starts udhcp6 needs to wait for it to be created.

I don't remember any issues getting the initial response though.
You will need to write a proper IPv6 version of the script that
actually does the work.
Also dhcp6 only gives you an address, the 'router advertise' stuff
also has to be enabled (autoconf can be left disabled, but you need
to be on a network where it would work).

David

> 
> > and the server doesn’t know how to reply (unreachable).
> 
> I would imagine server can/should reply with a broadcast, right?
> 
> > My link local address is set to:
> > inet6 fe80::da80:39ff:fed8:90a0  prefixlen 64  scopeid 0x20
> > but yet udhcpc6 seems to put just  ::  in for the source address.
> 
> But what if the iface has no link-local address either?
> Then udhcpc6 would have no choice but leave it not filled.
> And a good server implementation should deal with such incoming packets.
> 
> > If there’s a better place to post this - please let me know.
> 
> CCed to busybox ML.
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

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

[David Laight]

From: busybox Dominique Martinet
> Sent: 16 November 2022 02:53
> 
> From: Dominique Martinet 
> 
> sed would currently not error if write failed when modifying a file.

> diff --git a/editors/sed.c b/editors/sed.c
> index 32a4b61f6d4c..31ade17477ca 100644
> --- a/editors/sed.c
> +++ b/editors/sed.c
> @@ -1639,7 +1639,10 @@ int sed_main(int argc UNUSED_PARAM, char **argv)
>  fchown(nonstdoutfd, statbuf.st_uid, statbuf.st_gid);
> 
>  process_files();
> -fclose(G.nonstdout);
> +if (fclose(G.nonstdout)) {
> +xfunc_error_retval = 4;  /* It's what gnu sed exits with... */
> +bb_simple_error_msg_and_die(bb_msg_write_error);
> +}
>  G.nonstdout = stdout;

Does that report an error if there is nothing currently buffered?
I think the best sequence is:

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;

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: Subject: [PATCH] ash: make 'sleep' a builtin

[David Laight]

One question is why?
Shell builtins (mostly) exist to speed things up.
But sleep doesn’t need speeding up.

David

From: busybox  On Behalf Of 
shawnland...@tutanota.com
Sent: 30 August 2022 02:25
To: Denys Vlasenko 
Cc: Busybox 
Subject: Re: Subject: [PATCH] ash: make 'sleep' a builtin



27 ago 2022, 10:36 por 
vda.li...@googlemail.com:
You forgot this:

//kbuild:lib-$(CONFIG_SLEEP) += sleep.o
+//kbuild:lib-$(CONFIG_ASH_SLEEP) += sleep.o
fixed

/* BB_AUDIT SUSv3 compliant */
/* BB_AUDIT GNU issues -- fancy version matches except args must be ints. */
diff --git a/include/libbb.h b/include/libbb.h
index abbc9ac59..129858e27 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1511,6 +1511,7 @@ int hush_main(int argc, char** argv)
IF_SHELL_HUSH(MAIN_EXTERNALLY_VISIBLE);
/* If shell needs them, they exist even if not enabled as applets */
int echo_main(int argc, char** argv) IF_ECHO(MAIN_EXTERNALLY_VISIBLE);
int printf_main(int argc, char **argv) IF_PRINTF(MAIN_EXTERNALLY_VISIBLE);
+int sleep_main(int argc, char **argv) IF_SLEEP(MAIN_EXTERNALLY_VISIBLE);
int test_main(int argc, char **argv)


Also, your patch doesn't apply, whitespace damaged by mail agent.
Please resend as attachment.
Hopefully this works.

On Sat, Aug 27, 2022 at 6:25 AM 
mailto:shawnland...@tutanota.com>> wrote:

This reduces the number of processes spawned when in an indefinate
do-nothing loop in a shell script.

Signed-off-by: Shawn Landden 
mailto:shawnland...@tutanota.com>>
---
shell/ash.c | 11 +++
1 file changed, 11 insertions(+)

diff --git a/shell/ash.c b/shell/ash.c
index 55c1034f5..eba7759e7 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -134,6 +134,11 @@
//config: default y
//config: depends on SHELL_ASH
//config:
+//config:config ASH_SLEEP
+//config: bool "sleep builtin"
+//config: default y
+//config: depends on SHELL_ASH
+//config:
//config:config ASH_HELP
//config: bool "help builtin"
//config: default y
@@ -10155,6 +10160,9 @@ static int FAST_FUNC printfcmd(int argc, char **argv) { 
return printf_main(argc,
#if ENABLE_ASH_TEST || BASH_TEST2
static int FAST_FUNC testcmd(int argc, char **argv) { return test_main(argc, 
argv); }
#endif
+#if ENABLE_ASH_SLEEP
+static int FAST_FUNC sleepcmd(int argc, char **argv) { return sleep_main(argc, 
argv); }
+#endif

/* Keep these in proper order since it is searched via bsearch() */
static const struct builtincmd builtintab[] = {
@@ -10217,6 +10225,9 @@ static const struct builtincmd builtintab[] = {
{ BUILTIN_SPEC_REG "return" , returncmd },
{ BUILTIN_SPEC_REG "set" , setcmd },
{ BUILTIN_SPEC_REG "shift" , shiftcmd },
+#if ENABLE_ASH_SLEEP
+ { BUILTIN_REGULAR "sleep" , sleepcmd },
+#endif
#if BASH_SOURCE
{ BUILTIN_SPEC_REG "source" , dotcmd },
#endif
--
2.36.1

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

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: Bash compatibility TODO

[David Laight]

From: Aaro Koskinen
> Sent: 27 June 2022 21:43
> 
> Hi all,
> 
> Looks like it's not possible to build mainline Linux anymore with the
> busybox as "bash". :-(

Have you posted that to LKML ?
They really ought to solve the problem a different way.

David

> 
> $ make
>   CC  scripts/mod/empty.o
> ./scripts/check-local-export: line 17: shopt: not found
> make[1]: *** [scripts/Makefile.build:249: scripts/mod/empty.o] Error 127
> make[1]: *** Deleting file 'scripts/mod/empty.o'
> make: *** [Makefile:1199: prepare0] Error 2
> 
> It seems the requirement is to support "lastpipe" option:
> 
>   # Run the last element of a pipeline in the current shell.
>   # Without this, the while-loop would be executed in a subshell, and
>   # the changes made to 'symbol_types' and 'export_symbols' would be lost.
>   shopt -s lastpipe
> 
> and I don't think these will work either:
> 
>   declare -A symbol_types
>   declare -a export_symbols
> 
> A.
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH v2] seedrng: limit poolsize to 256 bytes and document flock() and fsync() usage

[David Laight]

From: Alexander Dahl
> Sent: 02 May 2022 06:15
> 
> Am Sat, Apr 30, 2022 at 03:12:11PM +0200 schrieb Denys Vlasenko:
> > Do you often pull power cords from machines you use for
> > somewhat important crypto operations, such as generating keys?
> > What are the chances that you also do it on a machine without RTC
> > clock (which would otherwise supply randomness
> > from the current time)?
> 
> FWIW … busybox is often used in embedded systems, and from my personal
> 10y+ experience in that field, I can safely assure you: customers and
> users pull power chords all the time and at absolutely unpredictable
> times.  Don't know if that's a real threat from a security point of
> view, but only looking at what developers do is not giving you the
> full picture.

The start of boot sequence is usually 'safe'.
Nothing external is likely to have access until towards the
end of it.
Someone with physical access can pull the power - but they
can do all sorts of other things as well.

The 'problem' is that systems with limited 'entropy' sources
need to carry over some 'randomness' between boots.

So you actually need to be updating the saved 'randomness'
every so often during normal running.
This is entirely different from initialising the PRNG
which should only be done one at boot time.

Now ideally the kernel would always 'stir' new entropy
into whatever it had before - so even giving is a few kb
of zeros wouldn't make things worse. But I suspect that
hasn't been the case.

Anyway while changing the saved randomness after loading
the PRNG is needed in case the power is cut not long after
the init scripts complete, it is much more useful to save
it at the end of the scripts (eg as an S99 script) when
there is a chance that some additional entropy has been
accrued.
Mild paranoia might be required when writing a normal file.
Although I suspect the safest is just to open the existing
file without O_TRUNC and rewrite exactly the same number of
bytes.

But what you really want to write is (probably) a
combination of the old saved value, the kernel entropy pool
and the kernel PRNG state.
(The latter two being hashes.)
But you need to do this without reloading the PRNG from the
entropy pool or feeding the old saved state into the entropy
pool.
I don't see how seedrng helps this operation, nor whether
the kernel code really lets it get suitable information.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH v2] seedrng: limit poolsize to 256 bytes and document flock() and fsync() usage

[David Laight]

From: Jason A. Donenfeld
> Sent: 30 April 2022 14:48
> 
> Hi Denys,
> 
> On Sat, Apr 30, 2022 at 3:12 PM Denys Vlasenko  
> wrote:
> > > +   /* The fsync() here is necessary for safety here, so that power 
> > > being pulled
> > > +* at the wrong moment doesn't result in the seed being used 
> > > twice by accident. */
> > > if ((unlink(filename) < 0 || fsync(dfd) < 0) && seed_len) {
> > > bb_perror_msg("can't%s seed", " remove");
> > > return -1;
> >
> > I don't understand the scenario mentioned here.
> > Let's say we removed fsync() above.
> > We read /var/lib/seedrng/seed.credit contents.
> > We unlink() it, but this change does not go to disk yet.
> > We seed the RNG.
> > We recreate /var/lib/seedrng/seed.credit with new contents,
> > but this change does not go to disk yet too.
> > We exit.
> > Only now, after we are done, RNG can be used for e.g. key generation.
> >
> > And only if the power gets pulled AFTER this key generation,
> > and the write cached data is still not on the disk
> > (for example, if you use ext4 or xfs, this won't happen since
> > they synchronously wait for both O_TRUNC truncations
> > and renames), the previous /var/lib/seedrng/seed.credit might
> > still exist and might get reused on the next boot,
> > and a new key can be generated from the same seed.
> >
> > Do you often pull power cords from machines you use for
> > somewhat important crypto operations, such as generating keys?
> > What are the chances that you also do it on a machine without RTC
> > clock (which would otherwise supply randomness
> > from the current time)?
> >
> > IOW: To me, this does not seem to be a realistic threat.
> > It's a theoretical one: you can concoct a scenario where it might happen,
> > but triggering it for real, even on purpose, would be difficult.
> 
> Again, stop charging steadfastly toward creating vulnerabilities
> because you don't understand things. The scenario is:
> 
> - RNG is seeded and credited using file A.
> - File A is unlinked but not fsync()d.
> - TLS connection does something and a nonce is generated.
> - System loses power and reboots.
> - RNG is seeded and credited using same file A.
> - TLS connection does something and the same nonce is generated,
> resulting in catastrophic cryptographic failure.
> 
> This is a big deal. Crediting seeds is not to be taken lightly.
> Crediting file-based seeds is _only_ safe when they're never used
> twice.

Using the same file twice is better than having nothing at all.
At least different systems use different values.
Unless you have a remote 'dos' attack that can crash the system
at exactly the right point in the boot sequence this is an
entirely 'academic' error.

What is much more likely is that the file where the entropy
is saved is just a memory overlay on top of a read-only image.

That is much more likely for an embedded system than any of
the 'failure' cases you've considered.

I also wonder how sane it is to do 'new_key = f(old_key)'.
That doesn't seem significantly better than using the same key.

For a really embedded system the only persistent storage
could easily be a small serial EEPROM with a very limited
number of write cycles.
This requires special code to read/write and care to avoid
hitting the write cycle count on a small number of memory cells.
No amount of faffing about with filesystem accesses will
help here at all.

There is also the case (that on my systems at least) udev
initialisation reads from /dev/[u]random well before the S20
script loads any saved entropy.
I've not tried to find out what the value is used for.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH v9] seedrng: import SeedRNG utility for kernel RNG seed files

[David Laight]

From: Denys Vlasenko
> Sent: 29 April 2022 17:17
> 
> On Wed, Apr 27, 2022 at 6:55 PM Jason A. Donenfeld  wrote:
> > On Wed, Apr 27, 2022 at 06:15:50PM +0200, Denys Vlasenko wrote:
> > > Can we replace all [s]size_t's with ints/unsigneds? We do not expect
> > > random pools anywhere near 4 gigabytes...
> >
> > Probably that's fine. Is the advantage to tossing out consistent types
> > worth it though? Does this actually save space? Since [s]size_t is
> > usually the word size, won't codegen not really change much?
> 
> For example, on x86-64, 32-bit insns are often shorter.

Provided you remember to use 'unsigned int' for array subscripts.
Otherwise you can get a lot of 'sign extend' instructions.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] ash: avoid some redirection oddities

[David Laight]

From: Ron Yorston
> Sent: 26 April 2022 16:10
> 
> A user reports:
> 
>I know this code is wrong but it shouldn't cause the shell to exit:
>echo text >&
>This instead cause the creation of a file:
>echo text >&99
> 
>I think it would be better if they report Bad file descriptor
> 
> The first case happens because save_fd_on_redirect() causes an exit
> when dup_CLOEXEC() returns any error other than EBADF.  Despite its
> name dup_CLOEXEC() actually uses fcntl(2) which returns EINVAL when
> the file descriptor is greater than the maximum allowed.
> 
> The second case is due to any numeric string longer than nine digits
> being treated as a file name rather than a number.  Bash doesn't
> do this.
...
> @@ -9635,7 +9634,7 @@ expredir(union node *n)
>   if (fn.list == NULL)
>   ash_msg_and_raise_error("redir error");
>  #if BASH_REDIR_OUTPUT
> - if (!isdigit_str9(fn.list->text)) {
> + if (!isdigit_str(fn.list->text)) {
>   /* >&file, not >&fd */
>   if (redir->nfile.fd != 1) /* 123>&file 
> - BAD */
>   ash_msg_and_raise_error("redir 
> error");
> @@ -11945,19 +11944,21 @@ static void
>  fixredir(union node *n, const char *text, int err)
>  {
>   int fd;
> + struct rlimit limit;
> 
>   TRACE(("Fix redir %s %d\n", text, err));
>   if (!err)
>   n->ndup.vname = NULL;
> 
> + getrlimit(RLIMIT_NOFILE, &limit);

Doing an extra system call seems suboptimal.
Why not just allow EINVAL as equivalent to EBADF later on?

>   fd = bb_strtou(text, NULL, 10);

Can't you do this conversion earlier and check the 'unconverted'
character for being '\0'?
Would save scanning the string twice.

> - if (!errno && fd >= 0)
> + if (!errno && fd >= 0 && fd < limit.rlim_cur - 1)
>   n->ndup.dupfd = fd;
>   else if (LONE_DASH(text))
>   n->ndup.dupfd = -1;
>   else {
>   if (err)
> - raise_error_syntax("bad fd number");
> + raise_error_syntax(strerror(EBADF));
>   n->ndup.vname = makename();
...

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: IPv6 support with busybox

[David Laight]

I’d check that the script that udhcpc6 runs to configure IPv6 is actually doing 
something sensible.
Especially if you are also trying to use IPv4 as well.

You’ll need a network that is running (IIRC) radv – otherwise you won’t get a 
prefix length (aka netmask) or any routes.
So if IPv6 ‘autoconf’ (SLAAS) won’t give you a valid IPv6 config then neither 
will dhcpc6.

David

From: busybox  On Behalf Of Vijay Khurdiya
Sent: 19 April 2022 13:05
To: busybox@busybox.net
Subject: Re: IPv6 support with busybox

Hi,
I installed busybox 1.29.3. I have tried to run tcpdump on my target to check 
on issue.
I am running udhcpc6 by command - (udhcpc6 -b -p /var/run/udhcpc.usb0.pid -i 
usb0)

Below is dump o/p. Please give me hints. Am I missing something?

---
17:15:27.615913 IP6 (hlim 1, next-header UDP (17) payload length: 62) 
fe80::f896:11ff:fe12:1314.dhcpv6-client > ff02::1:2.dhcpv6-server: [udp sum ok] 
dhcp6 solicit (xid=fdce7c (elapsed-time 65) (IA_NA IAID:986346022 T1:0 T2:0) 
(option-request DNS-server DNS-search-list) (Client-FQDN) (client-ID hwaddr 
type 1 fa9611121314))

On Tue, Apr 19, 2022 at 2:30 PM Vijay Khurdiya 
mailto:vijay.khurd...@gmail.com>> wrote:
Hi,
I would like to know from which version IPv6 support is available under 
busybox. I learned there is separate dameon  (udhcpc6) for IPv6.

I need to get an IPv6 address for Cellular network (M2M SIM).

Thanks

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH v8] seedrng: import SeedRNG utility for kernel RNG seed files

[David Laight]

From: Bernhard Reutner-Fischer
> Sent: 19 April 2022 09:32
> 
> On Tue, 12 Apr 2022 21:07:36 +0200
> "Jason A. Donenfeld"  wrote:
> 
> > Hi Bernhard,
> >
> > On Tue, Apr 12, 2022 at 8:37 PM Bernhard Reutner-Fischer
> >  wrote:
> > >
> > > Hi Jason!
> > > I'm a bit surprised that even if i give -n the seed is moved to
> > > seed.credit. The next boot/run will find the now creditable seed and
> > > happily add it, IIUC, despite i wanted it to not be credited?
> > > Is this intentional?
> >
> > Yes. You misunderstand the purpose of the utility. It creates a
> > creditable seed when the kernel is able to produce safe random
> > numbers. In that case, the creditability or non-creditability of the
> > previous seed does not matter.
> 
> So to go back to the underlying issue.
> In v8 we cannot guarantee that we really got a seed from
> the kernel, let alone trustworthy. It could have been data from about
> any file, including /dev/zero or other unhelpful, no-random data. I
> think that adding and crediting a block of 0 is not what we want.
> 
> Ignoring interference from other processes with CAP_SYS_ADMIN, to
> somewhat tighten this up, we'd need to open /dev/random once and do our
> stuff with this rnd_fd, including an fstat to ensure we are really
> reading from the random character device 1,8 or chardev 1,9 for
> urandom. [Can we ioctl on urandom to RNDADDENTROPY to the pool btw? ah
> yes, we can; only read is different in fops. That's handy and
> simplifies the flow.]

Does any of that matter at all?
If anything can change what the startup script/program does then they
can do something completely different instead.

The only 'problem' is the static one where someone has built a kernel
with an inappropriate set of options or is running the startup scripts
in the wrong order.
 
One thing I have noticed is that the first message about the rng not
being initialised comes (IIRC) from the udev script.
I think this means the S20 is too late to seed the rng.
(Or the used code shouldn't be trying to get random numbers.0

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: Question about isolating and re-using code from busybox

[David Laight]

From: Bernhard Reutner-Fischer
> Sent: 04 March 2022 13:38
> 
> On 4 March 2022 07:48:58 CET, Emmanuel Deloget  wrote:
> >Hello Tim,
> >
> >it seems to me that it should not be that complicated, as the tar
> >handling code is mostly encapsulated into a library of its own.
> >
> >See https://git.busybox.net/busybox/tree/archival/libarchive for
> >futher reference.
> 
> I think we provide means to build busybox as shared library. Iff this ist 
> still in there, you could
> simply link against that.
> 
> Of course you have to be aware that we do not guarantee a stable library API. 
> But that said, it gets
> changed seldomly if at all.

I'd worry about resource leaks if the process doesn't exit
after running an applet.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: suspected bug in timeout command

[David Laight]

From: Denys Vlasenko
> Sent: 01 March 2022 22:40
> 
> On Tue, Mar 1, 2022 at 11:15 PM David Laight  wrote:
> > From: Denys Vlasenko
> > > and a separate "spawn 40k 'sleep 0.03'" loop
> > > seems to indicate that openat(fd, ".") on the exited
> > > /proc/PID fails, and continues to fail
> > > even if another process with this PID exists
> > > again (pid was reused).
> >
> > Which kernel?
> > That may not fail on pre-pidfd kernels.
> > There were some discussions on the linux kernel mailing list
> > that I half followed.
> 
> 4.12.0-0.rc3.git0.2.fc27.x86_64

Reasonably old then.
I could check on a 3.10 RHEL7 kernel.

But any busybox fix is likely to run on a newish kernel.
Not some old distro kernel.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: suspected bug in timeout command

[David Laight]

From: Steffen Nurpmeso
> Sent: 01 March 2022 17:49
...
> 
> David Laight wrote in
>  :
>  |From: Denys Vlasenko
>  |> Sent: 01 March 2022 16:40
>  |> On Tue, Feb 15, 2022 at 12:31 PM Rob Landley  wrote:
>  |>> On 2/14/22 10:09 AM, Roberto A. Foglietta wrote:
>  ...
>  |> My memory is hazy on this, but IIRC kernel also actually has some
>  |> defensive code to not immediately reuse pids which just died.
>  |
>  |The Linux krnel only has protection for code inside the kernel.
>  |Basically there is a ref-counted structure that you need to send the
>  |signal - not the pid itself.
>  |I can't quite remember whether the pid itself can be reused even before
>  |that structure is freed.
>  |
>  |NetBSD does guarantee not to reuse a pid for a reasonable number
>  |of forks after a process exits.
> 
> ...which might be fruitless with 16-bit pids, define "reasonable".

Nothing stops pids going beyond 16 bits.
(Apart from some of the very old emulations.)
Much like nothing really requires the pid allocator to even start
allocating linearly (after giving init pid 1) - but that is
rather expected.

I can't remember at which point it went above 32767.
Was over 20 years ago when I wrote it :-)

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: suspected bug in timeout command

[David Laight]

From: Denys Vlasenko
> Sent: 01 March 2022 21:57
> 
> On Tue, Mar 1, 2022 at 5:39 PM Denys Vlasenko  
> wrote:
> > Meanwhile: what "timeout" is doing is it tries to get out
> > of the way of the PROG to be launched so that timeout's parent
> > sees PROG (not timeout) as a child. E.g. it can send signals
> > to it, get waitpid notifications if PROG has been stopped
> > with a signal, and such.
> >
> > And PROG also has no spurious "timeout" child.
> > "timeout" exists as an orphaned granchild.
> >
> > Let's go with a solution with fd opened to /proc/PID?
> 
> This little test:
> 
> int fd = open("/proc/self", O_RDONLY);
> int parent = getpid();
> int pid = fork();
> if (pid) { //parent
> sleep(1);
> exit(0);
> }
> sleep(2);
> printf("openat:%d\n", openat(fd, ".", O_RDONLY));
> while (openat(fd, ".", O_RDONLY) == -1)
> continue;
> 
> and a separate "spawn 40k 'sleep 0.03'" loop
> seems to indicate that openat(fd, ".") on the exited
> /proc/PID fails, and continues to fail
> even if another process with this PID exists
> again (pid was reused).

Which kernel?
That may not fail on pre-pidfd kernels.
There were some discussions on the linux kernel mailing list
that I half followed.

But it is probably a lot better that anything else at least
for the current kernels that people are probably going to use.

(Apart from those of us who have to build release binaries
on old systems in order that customers can run them on the
old distros they like to use)

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: suspected bug in timeout command

[David Laight]

From: Denys Vlasenko
> Sent: 01 March 2022 16:40
> 
> On Tue, Feb 15, 2022 at 12:31 PM Rob Landley  wrote:
> > On 2/14/22 10:09 AM, Roberto A. Foglietta wrote:
> > >  However, if this bug shows-up, probably it means that the system has
> > > a lot of processes running and a lot of processes created and
> > > destroyed compared to the max PID available. Thus, the system might be
> > > incorrectly configured compared with its typical usage which probably
> > > is the main reason because nobody complained before.
> >
> > Nah, a shell script can spin through an awful lot of PIDs pretty fast, and 
> > if
> > you're doing a -j 8 build that has a lot of script snippets (let alone 
> > parallel
> > autoconf etc) vs something with say a 10 second timeout?
> 
> I try the below and it seems to be able to spawn "only"
> ~1500 processes/second.
> 
> $ time sh -c 'i=3; while test $((--i)) != 0; do sleep 0 & done 
> 2>/dev/null'
> real0m19.190s
> user0m23.062s
> sys0m6.732s
> 
> My memory is hazy on this, but IIRC kernel also actually has some
> defensive code to not immediately reuse pids which just died.

The Linux krnel only has protection for code inside the kernel.
Basically there is a ref-counted structure that you need to send the
signal - not the pid itself.
I can't quite remember whether the pid itself can be reused even before
that structure is freed.

NetBSD does guarantee not to reuse a pid for a reasonable number
of forks after a process exits.

> It's interesting to find out why pids are reused that fast on the
> affected system.

They are allocated by a simple numeric scan for a free pid.
So it depends on where the scan is and the pid being freed.
It can be reused for the very next fork().

> Meanwhile: what "timeout" is doing is it tries to get out
> of the way of the PROG to be launched so that timeout's parent
> sees PROG (not timeout) as a child. E.g. it can send signals
> to it, get waitpid notifications if PROG has been stopped
> with a signal, and such.

I also believe it is only checking the pid every (few?) seconds.

> And PROG also has no spurious "timeout" child.
> "timeout" exists as an orphaned granchild.
> 
> Let's go with a solution with fd opened to /proc/PID?

I think you need to verify some part of the process state.
Especially for pre-pidfd kernels.
Probably the process start time.
If that changes the pid has been reused.

That gets the timing window down to 'we checked it was the
right process', but the pid was reused before we could send
the signal.
It also requires the process to exit on exactly its timeout.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH v2] ash: use memmove instead of mempcpy in subevalvar

[David Laight]

From: busybox
> Sent: 26 February 2022 18:53
> 
> While investigating a sporadic crash issue relating to variable substitution 
> in
> Alpine Linux, we managed to get a reliable crash when building BusyBox with 
> ASan,
> due to the source and destination overlapping for mempcpy, which resulted in
> sporadic data corruption outside ASan.
> 
> Per POSIX, memcpy is not allowed to overlap source and destination, as mempcpy
> is a GNU-specific extension to mempcpy, the same semantics can be assumed.
> Accordingly, we use memmove instead, which does not have this limitation.
> 
> v2: Forgot to emulate mempcpy's dest+size return value, fixed.
> 
> Signed-off-by: Ariadne Conill 
> ---
>  shell/ash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/shell/ash.c b/shell/ash.c
> index adb0f223a..056954059 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -7187,7 +7187,7 @@ subevalvar(char *start, char *str, int strloc,
>   len = orig_len - pos;
> 
>   if (!quotes) {
> - loc = mempcpy(startp, startp + pos, len);
> + loc = memmove(startp, startp + pos, len) + len;

I'd just not rely on the return value at all.
Juat add:
loc = startp + len;
before of after the call.

I'm actually intrigued that ash has picked up a glibc function
I thought it was portable?

The 'best' ash bug (which might now be fixed in the main sources)
was running on the beginning of an on-stack buffer when removing
the '\n' from the end of long $() substitutions.
It usually just failed to remove a '\n', but it could remove an
extra character - most likely on BE systems.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: suspected bug in timeout command

[David Laight]

Yes it does.
But it is unlikely to happen in the short time between some kind of test
that the process is the right one, and sending the signal.
But hoping to see the process ‘gone’ on a 1 second poll is pretty useless.

So if the process’s ‘start time’ is found (before any of the forks) that can
be passed through to the grandchild and checked every second.
Then you are very unlikely to kill the wrong process.

If you also pass through the open directory fd to “/proc/self” and use openat()
to open whichever /proc/xx/yy file is needed then I think with a later kernel
you’ll get an error is the process has died (and the pid reused) and you can
also use it to send the signal – avoiding the pid reuse race.

Just a smop.

David


Im in not wrong linux use a simple allocation method for pid’d were it just set 
the next pid to last pid allocated + 1. Then check if its free and if not keep 
adding until found free one. A process can be created few minutes before it 
exits and meanwhile the pid’s are continuing and wrap around to the point that 
at the moment it release is the exact same moment when the next_pid is the same 
one as the process that just exits.

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: suspected bug in timeout command

[David Laight]

From: Michael Conrad
> Sent: 12 February 2022 12:59
> 
> On 2/12/22 07:38, Michael Conrad wrote:
> > Correctly using pidfd *still* requires that you be the parent process,
> > else the child could get reaped and replaced before the pidfd is
> > created.  As far as I can tell, the only purpose of pidfd is for
> > waking on poll() instead of using signals, which is orthagonal to this
> > problem.

Even if the pidfd can't be passed down, it lets you verify the process
information and then send the signal.

> > I haven't looked at the source in busybox yet, but it boggles my mind
> > that it wouldn't just be a simple fork+alarm+waitpid because that is
> > literally the least code implementation, and race-free.
> >
> > -Mike C
> >
> Sorry for being lazy.  I looked at the source and this is the reason:
> 
> /* We want to create a grandchild which will watch
>   * and kill the grandparent. Other methods:
>   * making parent watch child disrupts parent<->child link
>   * (example: "tcpsvd 0.0.0.0 1234 timeout service_prog" -
>   * it's better if service_prog is a child of tcpsvd!),
>   * making child watch parent results in programs having
>   * unexpected children.    */
> 
> I don't follow this reasoning.  Does "disrupts the parent<->child link"
> just about sending signals?  If the timeout app relays all signals from
> itself to the child, what remaining problems would exist?

In that case you can pass 'verification information' through to
the grandchild.

It could be an open fd of "/proc/self" - which allows the non-racy
kill on recent kernels.
But other information would allow the timing window be minimalised on
older kernels.

ISTR that on older kernels an open fd to "proc/nn" always refers to the
current process with pid nn. But the actual behaviour is worth checking.
I think newer kernels will fail any reads after the process has exited.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: suspected bug in timeout command

[David Laight]

From: Raffaello D. Di Napoli
> Sent: 12 February 2022 01:33
> 
> 
> On 2/11/22 16:22, Rob Landley wrote:
> > On 2/9/22 11:12 AM, Baruch Siach wrote:
> >> Hi Sun,
> >>
> >> On Wed, Feb 09 2022, סאן עמר wrote:
> >>> Hi, I'm using busybox for a while now (v1.29.2). and I had an issue with 
> >>> a sigterm send randomly
> to a process of mine. I debugged it until I found
> >>> it from the timeout process which was assigned before to another process 
> >>> with the same pid. (i'm
> using a lot of timeouts for a lot of jobs)
> >>> so i looked at the code, "timeout.c" file where it sleep for 1 second in 
> >>> each iteration then check
> the timeout status. I suspect at this time the
> >>> process timeout monitoring is terminated, but another one with the same 
> >>> pid is already created.
> which creates unwanted timeout.
> >>>
> >>> There is a comment in there about sleep for "HUGE NUM" will probably 
> >>> result in this issue, but I
> can't see why it won't occur also in the current
> >>> case.
> >>>
> >>> there is no change of this behaviour in the latest master.
> >>> i would appreciate any help, sun.
> >> Any reference to PID number is inherently racy.
> > Not between parent and child.
> 
> Except in BB’s timeout, the relationship is not parent/child :)
> 
> Much to my surprise, I’ll say that. When I read the bug report the other
> day, I thought to myself well, this one ought to be easy to fix. But no,
> there’s no SIGCHLD to be handled, no relationship between processes to
> be leveraged.
> 
> I don’t think this bug can be fixed without a near-complete rewrite, or
> without doing a lot of procfs digging to really validate the waited-on
> process, since kill(pid, 0) only validates a pid, not a process.

And Linux uses a strict 'next free pid' algorithm for new processes
so the is no guard time between a process exiting and its pid being reused.
This problem was 'fixed' inside the kernel by using a small structure
instead of the pid itself - but that didn't help userspace (or even some 
drivers).
By comparison NetBSD uses the high bits of the pid as a 'generation number'
and so guarantees that a pid won't be reused for some time (a few thousand 
forks).

You can use the process start time (I think it is in /proc/pid/stat)
to validate the process just before the kill().
That leaves a very small timing window that it is hard to avoid
without using pidfd.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH v2] Improve support for long options to grep

[David Laight]

From: Walter Lozano
> Sent: 26 January 2022 14:14
> 
> In order to improve compatibility with GNU grep improve support for long
> options to busybox grep.

You've broken backwards compatibility by removing --color from
default builds.
There is also unnecessary duplication of source code.

Just use a #define and C string concatenation to add the extra
long option strings.

David

> 
> Signed-off-by: Walter Lozano 
> 
> ---
> 
> Changes in v2:
> - Decouple FEATURE_GREP_CONTEXT from LONG_OPTS.
> 
>  findutils/grep.c | 43 +--
>  1 file changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/findutils/grep.c b/findutils/grep.c
> index 0b72812f1..4ae070a01 100644
> --- a/findutils/grep.c
> +++ b/findutils/grep.c
> @@ -720,17 +720,56 @@ int grep_main(int argc UNUSED_PARAM, char **argv)
> 
>   /* do normal option parsing */
>  #if ENABLE_FEATURE_GREP_CONTEXT
> +#if !ENABLE_LONG_OPTS
>   /* -H unsets -h; -C unsets -A,-B */
> - opts = getopt32long(argv, "^"
> + opts = getopt32(argv, "^"
>   OPTSTR_GREP
>   "\0"
>   "H-h:C-AB",
> - "color\0" Optional_argument "\xff",
>   &pattern_head, &fopt, &max_matches,
>   &lines_after, &lines_before, &Copt
>   , NULL
>   );
> +#else
> + static const char grep_longopts[] ALIGN1 =
> + "with-filename\0"No_argument "H"
> + "no-filename\0"  No_argument "h"
> + "line-number\0"  No_argument "n"
> + "files-without-match\0"  No_argument "L"
> + "files-with-matches\0"   No_argument "l"
> + "count\0"No_argument "c"
> + "only-matching\0"No_argument "o"
> + "quiet\0"No_argument "q"
> + "silent\0"   No_argument "q"
> + "invert-match\0" No_argument "v"
> + "no-messages\0"  No_argument "s"
> + "recursive\0"No_argument "r"
> + "ignore-case\0"  No_argument "i"
> + "word-regexp\0"  No_argument "w"
> + "line-regexp\0"  No_argument "x"
> + "fixed-strings\0"No_argument "F"
> + "extended-regexp\0"  No_argument "E"
> + "null-data\0"No_argument "z"
> + "max-count\0"Required_argument   "m"
> + "after-context\0"Required_argument   "A"
> + "before-context\0"   Required_argument   "B"
> + "context\0"  Required_argument   "C"
> + "regexp\0"   Required_argument   "e"
> + "file\0" Required_argument   "f"
> + "color\0"Optional_argument   "\xff"
> + ;
> 
> + /* -H unsets -h; -C unsets -A,-B */
> + opts = getopt32long(argv, "^"
> + OPTSTR_GREP
> + "\0"
> + "H-h:C-AB",
> + grep_longopts,
> + &pattern_head, &fopt, &max_matches,
> + &lines_after, &lines_before, &Copt
> + , NULL
> + );
> +#endif
>   if (opts & OPT_C) {
>   /* -C unsets prev -A and -B, but following -A or -B
>* may override it */
> --
> 2.25.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] top '-c' switch to enable output of SMP cpu lines.

[David Laight]

From: Jonathan Sambrook
> Sent: 25 January 2022 17:44
> 
> This allows access to SMP cpu lines for batch output, as well as
> displaying SMP info in interactive mode without having to press '1'.
> 
> My motivation here is to be able to access the SMP information when
> graphing BusyBox top batch output with topplot
..
> +//usage: IF_FEATURE_TOP_SMP_CPU(
> +//usage:   "\n"" -c  Same as '1' key"
> +//usage: )

Better to just say 'One line for each cpu'.

...
> +#if ENABLE_FEATURE_TOP_SMP_CPU
> + if (col & OPT_c) {
> + cpu_jif = cpu_prev_jif = NULL;
> +num_cpus = 0;

Whitespace manged...

> + smp_cpu_info = !smp_cpu_info;
> + get_jiffy_counts();
> + }
> +#endif

Is this block needed - I suspect the main loop can be made to DTRT.

You also probably want the 'IRIX mode' patches I posted a few months back.
Otherwise on a system with a lot of cpu the per-process cpu time is
always very small.

They may have got lost

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] Improve support for long options to grep

[David Laight]

From: Walter Lozano
> Sent: 24 January 2022 14:48
> 
> In order to improve compatibility with GNU grep improve support for long
> options to busybox grep.

Why?
Does anyone ever actually type these long strings?
Looks like bloat to me.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] XXXsum: handle binary sums with " " in the path

[David Laight]

From: Emanuele Giacomelli
> Sent: 07 January 2022 11:00
> 
> If a line specifies a binary checksum whose path contains two adjacent
> spaces, when checking digests with -c the two spaces will be used as the
> separator between the digest and the pathname instead of " *", as shown:
> 
> $ echo foo > "/tmp/two  spaces"
> $ md5sum -b "/tmp/two  spaces"   # This is GNU md5sum
> d3b07384d113edec49eaa6238ad5ff00 */tmp/two  spaces
> $ md5sum -b "/tmp/two  spaces" | ./busybox md5sum -c
> md5sum: can't open 'spaces': No such file or directory
> spaces: FAILED
> md5sum: WARNING: 1 of 1 computed checksums did NOT match
> 
> The patch looks for both markers and uses the one that is closer to the
> start.
> ---
>  coreutils/md5_sha1_sum.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/coreutils/md5_sha1_sum.c b/coreutils/md5_sha1_sum.c
> index 3b389cb6b..b7c1495d2 100644
> --- a/coreutils/md5_sha1_sum.c
> +++ b/coreutils/md5_sha1_sum.c
> @@ -298,12 +298,17 @@ int md5_sha1_sum_main(int argc UNUSED_PARAM, char 
> **argv)
>   while ((line = xmalloc_fgetline(pre_computed_stream)) 
> != NULL) {
>   uint8_t *hash_value;
>   char *filename_ptr;
> + char *filename_ptr_bin;
> 
>   count_total++;
>   filename_ptr = strstr(line, "  ");
>   /* handle format for binary checksums */
> + filename_ptr_bin = strstr(line, " *");
>   if (filename_ptr == NULL) {
> - filename_ptr = strstr(line, " *");
> + filename_ptr = filename_ptr_bin;
> + } else {
> + filename_ptr = (filename_ptr_bin != 
> NULL && filename_ptr_bin <
> filename_ptr) ?
> + filename_ptr_bin : filename_ptr;

Wouldn't it be easier to just use strchr() to find a ' ' and then
check the next character for ' ' or '*' ?

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH] printf: allow 0 as a flag and allow multiple flags

[David Laight]

From: Denys Vlasenko
> Sent: 17 December 2021 20:10
> 
> On Thu, Dec 16, 2021 at 12:26 PM Ron Yorston  wrote:
> > The '%' character in a format specification may be followed by
> > one or more flags from the list "+- #0".  BusyBox printf didn't
> > support the '0' flag or allow multiple flags to be provided.
> > As a result the formats '%0*d' and '%0 d' were considered to be
> > invalid.
> >
> > The lack of support for '0' was pointed out by Andrew Snyder on the
> > musl mailing list:
> >
> >https://www.openwall.com/lists/musl/2021/12/14/2
> >
> > function old new   delta
> > printf_main  860 891 +31
> > .rodata99281   99282  +1
> > --
> > (add/remove: 0/0 grow/shrink: 2/0 up/down: 32/0)   Total: 32 
> > bytes
> >
> > Signed-off-by: Ron Yorston 
> > ---
> >  coreutils/printf.c |  8 +---
> >  testsuite/printf.tests | 10 ++
> >  2 files changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/coreutils/printf.c b/coreutils/printf.c
> > index dd94c8ade..e5cf5f942 100644
> > --- a/coreutils/printf.c
> > +++ b/coreutils/printf.c
> > @@ -313,9 +313,11 @@ static char **print_formatted(char *f, char **argv, 
> > int *conv_err)
> > }
> > break;
> > }
> > -   if (*f && strchr("-+ #", *f)) {
> > -   ++f;
> > -   ++direc_length;
> > +   if (*f) {
> > +   while (strchr("-+ #0", *f)) {
> > +   ++f;
> > +   ++direc_length;
> > +   }
> 
> This can walk off the end of "f", doesn't it?
> [strchr("anything", '\0') is not NULL]
> I propose
> 
> while (*f && strchr("-+ #0", *f)) {
> ++f;
> ++direc_length;
> }

Or:
tmp = strspn(f, "-+ #0");
direc_length += tmp - f;
f = tmp;

Except that I'd have thought it would need to remember which flags are set.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: [PATCH 3/3] nslookup: query class: remove support for class HS and ANY

[David Laight]

From: Moraes Holschuh
> Sent: 08 December 2021 14:20
> 
> Remove the support for classes HS (Hesiod) and ANY (wildcard), and with
> these gone, also remove the nice, table-driven support for specifying
> query classes and use an if(strcasecmp()) hardcoded tree instead.
> 
> This reduces some functionality and results in uglier code, but it does
> decrease the size of the "query class" support.
> 
> NOTE: This change should not be applied if the decrease in size is not
> considered worth the functionality and code quality loss.  I personally
> feel it is *not* worth the 19 bytes reduction in size.

Personally I suspect that modern systems aren't as memory/disc
constrained as those when busybox was first written.
So additional code in a paged executable doesn't matter as much.
Very small systems are actually unlikely to nslookup at all.

This also applies to some of the other applets.
The defaults could include all the 'expected' options that only
increase the code size by a few hundred bytes.
Indeed all the relevant options could be hidden behind a big
'optimise applets for size' option.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: Tar component in busybox version 1.34.1 has a memory leak bug when trying to unpack a tar file.

[David Laight]

From: busybox  On Behalf Of Ping Lee
> Sent: 01 December 2021 01:42

> It seems that I found a bug on busybox version 1.34.1:
> In libbb/xfuncs_printf.c:50, malloc twice for archive_handle and 
> archive_hadle->fileheader with 184 and 72 bytes heap space.

> Back to tar_main function, the two 
> pointers(tar_handle，tar_handle->file_header) hasn't been freed when return.

It can't matter it is a short-lived program that is going to exit.
It would only be a problem if the code were in a loop.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: return type demotion remarks

[David Laight]

From: Bernhard Reutner-Fischer
> Sent: 28 November 2021 19:37
> 
> On Sun, 28 Nov 2021 18:00:31 +0000
> David Laight  wrote:
> 
> > From: Bernhard Reutner-Fischer
> > > Sent: 27 November 2021 20:57
> > >
> > > Hi!
> > >
> > > I'm attaching a color log of some spots that may benefit of using
> > > narrower return types; See "could return".
> >
> > My mailer won't expand a .gz file .
> 
> yea, it was too big to send uncompressed, sorry for that.
> >
> > However making function arguments and return values smaller
> > than a normal cpu register (long is probably better than int!)
> > is likely to increase code size.
> 
> Arches will promote types as they see fit anyway.

The problem is that the values start needing to be masked to the
smaller type, not that they get promoted.

> I'm attaching something that i would _not_ apply since it was based off
> the user-visible fix-it not taking into account the shifts needed in
> the callers.
> Lessons learned in this v0.0:
> - iterate_on_dir callback should be void
> - find ACTF could use a narrower type
> - functors are a bit of a pain ;)
> cheers,

Try using outlook :-(
It really only wants to execute attachments, not display them.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

RE: return type demotion remarks

[David Laight]

From: Bernhard Reutner-Fischer
> Sent: 27 November 2021 20:57
> 
> Hi!
> 
> I'm attaching a color log of some spots that may benefit of using
> narrower return types; See "could return".

My mailer won't expand a .gz file .

However making function arguments and return values smaller
than a normal cpu register (long is probably better than int!)
is likely to increase code size.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH v2] ed: add support for -p command-line option as mandated by POSIX

[David Laight]

From: soe...@soeren-tempel.net
> Sent: 21 November 2021 11:25
> 
> The POSIX.1-2008 specification of ed(1) mandates two command-line
> options: -p (for specifying a prompt string) and -s (to suppress writing
> of byte counts). This commit adds support for the former. Furthermore,
> it also changes the default prompt string to an empty string (instead
> of ": ") since this is also mandated by POSIX:
> 
>   -p string Use string as the prompt string when in command mode.
> By default, there shall be no prompt string.
> 
> Support for the remaining -s option will be added in a separate commit
> since it requires a general restructuring of error handling in Busybox
> ed.
...
> @@ -790,7 +792,7 @@ static void doCommands(void)
>* 0  on ctrl-C,
>* >0 length of input string, including terminating '\n'
>*/
> - len = read_line_input(NULL, ": ", buf, sizeof(buf));
> + len = read_line_input(NULL, prompt, buf, sizeof(buf));

Actually it is probably cleaner to put the NULL test when prompt is used:

len = read_line_input(NULL, prompt ? prompt : "", buf, sizeof(buf));

Saves all the faffing about.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

RE: [PATCH] ed: add support for -p command-line option as mandated by POSIX

[David Laight]

From: soe...@soeren-tempel.net
> Sent: 20 November 2021 17:17
> 
> The POSIX.1-2008 specification of ed(1) mandates two command-line
> options: -p (for specifying a prompt string) and -s (to suppress writing
> of byte counts). This commit adds support for the former. Furthermore,
> it also changes the default prompt string to an empty string (instead
> of ": ") since this is also mandated by POSIX:
> 
>   -p string Use string as the prompt string when in command mode.
> By default, there shall be no prompt string.
> 
...
> - if (argv[1]) {
> - fileName = xstrdup(argv[1]);
> + opt = getopt32(argv, "p:", &prompt);
> + if (!(opt & 0x1))
> + prompt = xstrdup(""); /* no prompt by default */

You shouldn't need the strdup().
I think you can even do:
if (!(opt & 1))
prompt = "";
because (IIRC and for historic reasons) quoted strings are char[] not const 
char[].
OTOH an explicit local static or a zero byte in the data area
might be more usual for busybox.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

re: [PATCH] Fix const pointer assignment trick on clang 9+

[David Laight]

If you are trying to tell the compiler that a pointer
(or other value) might change and it mustn't reload it (etc)
then this might work:

#define LAUNDER(val_in, val_out) \
asm volatile ( "" : "&r" (val_out) : "0" (val_in))

That is (more or less) equivalent to:
val_out = (typeof(val_out))val_in;
except that the compiler can't track the value.
(assuming it doesn't 'peek inside' the asm.)

David
 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

[PATCH v2 2/2] top: Add 'IRIX mode'

[David Laight]

Add IRIX mode to process cpu% calculation.

For historic reasons top has two different ways of calculation the
process cpu% in a multi-cpu system.
Add 'IRIX mode' where a single-threaded process in an infinite loop
shows 100% and a multi-threaded program can exceed 100%.
The default remains 'Solaris mode' (even for -H) where the maximum
cpu% for a single thread is 100/num_cpus.

Signed-off-by: David Laight 

---


--- b/top.c 2021-11-12 11:25:43.849264597 +
+++ c/top.c 2021-11-12 11:36:49.510234350 +
@@ -165,6 +165,7 @@
 #endif
 #if ENABLE_FEATURE_TOP_SMP_CPU
smallint smp_cpu_info; /* one/many cpu info lines? */
+   smallint irix_mode;/* 100% => one cpu busy */
 #endif
unsigned lines;  /* screen height */
 #if ENABLE_FEATURE_TOP_INTERACTIVE
@@ -201,6 +202,7 @@
 #define sort_field   (G.sort_field)
 #define inverted (G.inverted  )
 #define smp_cpu_info (G.smp_cpu_info  )
+#define irix_mode(G.irix_mode )
 #define initial_settings (G.initial_settings  )
 #define sort_function(G.sort_function )
 #define prev_hist(G.prev_hist )
@@ -222,8 +224,9 @@
OPT_n = (1 << 1),
OPT_b = (1 << 2),
OPT_H = (1 << 3),
-   OPT_m = (1 << 4),
-   OPT_EOF = (1 << 5), /* pseudo: "we saw EOF in stdin" */
+   OPT_I = (1 << 4),
+   OPT_m = (1 << 5),
+   OPT_EOF = (1 << 6), /* pseudo: "we saw EOF in stdin" */
 };
 #define OPT_BATCH_MODE (option_mask32 & OPT_b)
 
@@ -311,6 +314,13 @@
return;
 #else
if (!smp_cpu_info) {
+   if (irix_mode && !num_cpus) {
+   jiffy_counts_t dummy_jif;
+   while (read_cpu_jiffy(fp, &dummy_jif) >= 4)
+   num_cpus++;
+   if (!num_cpus)
+   irix_mode = 0;
+   }
fclose(fp);
return;
}
@@ -599,6 +609,27 @@
return meminfo[MI_MEMTOTAL];
 }
 
+#if ENABLE_FEATURE_TOP_DECIMALS
+# define UPSCALE 1000
+# define STAT_BUF(buf_name) char buf_name[8];
+# define SHOW_STAT(name, buf) fmt_proc_pc(name, buf)
+# define FMT "%s"
+static NOINLINE const char *fmt_proc_pc(unsigned value, char *buf)
+{
+   if (value >= 1000)
+   snprintf(buf, 16, "%5u", value/10);
+   else
+   snprintf(buf, 16, "%3u.%c", value/10, '0' + (value % 10));
+   return buf;
+}
+
+#else
+# define UPSCALE 100
+# define STAT_BUF(buf_name)
+# define SHOW_STAT(name, buf) name
+# define FMT "%4u%%"
+#endif
+
 static NOINLINE void display_process_list(int lines_rem, int scr_width)
 {
top_status_t *s;
@@ -615,21 +646,13 @@
" COMMAND");
lines_rem--;
 
-#if ENABLE_FEATURE_TOP_DECIMALS
-# define UPSCALE 1000
-# define CALC_STAT(name, val) div_t name = div((val), 10)
-# define SHOW_STAT(name) name.quot, '0'+name.rem
-# define FMT "%3u.%c"
-#else
-# define UPSCALE 100
-# define CALC_STAT(name, val) unsigned name = (val)
-# define SHOW_STAT(name) name
-# define FMT "%4u%%"
-#endif
-
 #if ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE
/* s->pcpu is in jiffies */
delta_jifs = cur_jif.total - prev_jif.total;
+#if ENABLE_FEATURE_TOP_SMP_CPU
+   if (irix_mode && num_cpus)
+   delta_jifs /= num_cpus;
+#endif
 #endif
 
/* Ok, all preliminary data is ready, go through the list */
@@ -641,9 +664,11 @@
char vsz_str_buf[8];
unsigned col;
 
-   CALC_STAT(pmem, (s->vsz * UPSCALE + total_memory/2) / 
total_memory);
+   unsigned pmem = (s->vsz * UPSCALE + total_memory/2) / 
total_memory;
+   STAT_BUF(pmem_buf)
 #if ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE
-   CALC_STAT(pcpu, (s->pcpu * UPSCALE + delta_jifs/2) / 
delta_jifs);
+   unsigned pcpu = (s->pcpu * UPSCALE + delta_jifs/2) / delta_jifs;
+   STAT_BUF(pcpu_buf)
 #endif
 
smart_ulltoa5(s->vsz, vsz_str_buf, " mgtpezy");
@@ -655,9 +680,9 @@
" ",
s->pid, s->ppid, get_cached_username(s->uid),
s->state, vsz_str_buf,
-   SHOW_STAT(pmem)
+   SHOW_STAT(pmem, pmem_buf)
IF_FEATURE_TOP_SMP_PROCESS(, 
s->last_seen_on_cpu)
-   IF_FEATURE_TOP_CPU_USAGE_PERCENTAGE(, 
SHOW_STAT(pcpu))
+   IF_FEATURE_TOP_CPU_USAGE_PERCENTAGE(, 
SHOW_STAT(pcpu, pcpu_buf))
);
if ((int)(scr_width - col) > 1)
read_cmdline(line_bu

[PATCH v2 1/2] top: remove 'cruft' the process display code.

[David Laight]

Simplify process cpu% and vsz% calculations.

Remove the code that avoided integer divides when generating the cpu%.
The loop will be dominated by the system calls to read the command line.
So even if the division is a code loop it is probably noise.

On Linux the process cpu time values are in 'jiffies' so there is
no need to re-scale the values based on the sum of the process times.
(This was already assumed because of issues with short lived processes.)

Signed-off-by: David Laight 

---


--- a/top.c 2021-11-11 09:49:08.165982660 +
+++ b/top.c 2021-11-12 11:25:43.849264597 +
@@ -139,7 +139,6 @@
unsigned long long usr, nic, sys, idle;
unsigned long long iowait, irq, softirq, steal;
unsigned long long total;
-   unsigned long long busy;
 } jiffy_counts_t;
 
 /* This structure stores some critical information from one frame to
@@ -292,8 +291,6 @@
if (ret >= 4) {
p_jif->total = p_jif->usr + p_jif->nic + p_jif->sys + 
p_jif->idle
+ p_jif->iowait + p_jif->irq + p_jif->softirq + 
p_jif->steal;
-   /* procps 2.x does not count iowait as busy time */
-   p_jif->busy = p_jif->total - p_jif->idle - p_jif->iowait;
}
 
return ret;
@@ -604,19 +601,10 @@
 
 static NOINLINE void display_process_list(int lines_rem, int scr_width)
 {
-   enum {
-   BITS_PER_INT = sizeof(int) * 8
-   };
-
top_status_t *s;
unsigned long total_memory = display_header(scr_width, &lines_rem); /* 
or use total_vsz? */
-   /* xxx_shift and xxx_scale variables allow us to replace
-* expensive divides with multiply and shift */
-   unsigned pmem_shift, pmem_scale, pmem_half;
 #if ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE
-   unsigned tmp_unsigned;
-   unsigned pcpu_shift, pcpu_scale, pcpu_half;
-   unsigned busy_jifs;
+   unsigned delta_jifs;
 #endif
 
/* what info of the processes is shown */
@@ -638,49 +626,10 @@
 # define SHOW_STAT(name) name
 # define FMT "%4u%%"
 #endif
-   /*
-* %VSZ = s->vsz/MemTotal
-*/
-   pmem_shift = BITS_PER_INT-11;
-   pmem_scale = UPSCALE*(1U<<(BITS_PER_INT-11)) / total_memory;
-   /* s->vsz is in kb. we want (s->vsz * pmem_scale) to never overflow */
-   while (pmem_scale >= 512) {
-   pmem_scale /= 4;
-   pmem_shift -= 2;
-   }
-   pmem_half = (1U << pmem_shift) / (ENABLE_FEATURE_TOP_DECIMALS ? 20 : 2);
-#if ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE
-   busy_jifs = cur_jif.busy - prev_jif.busy;
-   /* This happens if there were lots of short-lived processes
-* between two top updates (e.g. compilation) */
-   if (total_pcpu < busy_jifs) total_pcpu = busy_jifs;
 
-   /*
-* CPU% = s->pcpu/sum(s->pcpu) * busy_cpu_ticks/total_cpu_ticks
-* (pcpu is delta of sys+user time between samples)
-*/
-   /* (cur_jif.xxx - prev_jif.xxx) and s->pcpu are
-* in 0..~64000 range (HZ*update_interval).
-* we assume that unsigned is at least 32-bit.
-*/
-   pcpu_shift = 6;
-   pcpu_scale = UPSCALE*64 * (uint16_t)busy_jifs;
-   if (pcpu_scale == 0)
-   pcpu_scale = 1;
-   while (pcpu_scale < (1U << (BITS_PER_INT-2))) {
-   pcpu_scale *= 4;
-   pcpu_shift += 2;
-   }
-   tmp_unsigned = (uint16_t)(cur_jif.total - prev_jif.total) * total_pcpu;
-   if (tmp_unsigned != 0)
-   pcpu_scale /= tmp_unsigned;
-   /* we want (s->pcpu * pcpu_scale) to never overflow */
-   while (pcpu_scale >= 1024) {
-   pcpu_scale /= 4;
-   pcpu_shift -= 2;
-   }
-   pcpu_half = (1U << pcpu_shift) / (ENABLE_FEATURE_TOP_DECIMALS ? 20 : 2);
-   /* printf(" pmem_scale=%u pcpu_scale=%u ", pmem_scale, pcpu_scale); */
+#if ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE
+   /* s->pcpu is in jiffies */
+   delta_jifs = cur_jif.total - prev_jif.total;
 #endif
 
/* Ok, all preliminary data is ready, go through the list */
@@ -692,9 +641,9 @@
char vsz_str_buf[8];
unsigned col;
 
-   CALC_STAT(pmem, (s->vsz*pmem_scale + pmem_half) >> pmem_shift);
+   CALC_STAT(pmem, (s->vsz * UPSCALE + total_memory/2) / 
total_memory);
 #if ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE
-   CALC_STAT(pcpu, (s->pcpu*pcpu_scale + pcpu_half) >> pcpu_shift);
+   CALC_STAT(pcpu, (s->pcpu * UPSCALE + delta_jifs/2) / 
delta_jifs);
 #endif
 
smart_ulltoa5(s->vsz, vsz_str_buf, " mgtpezy");

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

[PATCH v2 0/2] top: Add 'IRIX mode'

[David Laight]

The busybox top always runs in 'Solaris mode' where the cpu% for
a process is a percentage across all the cpu.
So a single threaded process in an infinite loop on a 16cpu
system only reports 8% cpu use.
In 'IRIX mode' it will report 100% - much more useful.
In IRIX mode a multi-threaded process can report over 100%.

When threads are displayed (-H) IRIX mode probably ought to be the
default - but I've not changed that.

Patch 1/2 removes a lot of 'cruft' from the process display that
was there to avoid two integer divides.
I actually doubt that was ever worth-while, the loop is probably
dominated by the system calls to read the process arguments.

Patch 2/2 adds IRIX mode.

Changes from v1 - add patch 1/2.

Both patches are diffs against the 1.33 release install.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

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

Add 'IRIX mode' to top.

[David Laight]

Add 'IRIX mode' to top.

On a multi-cpu system 'IRIX mode' displays a singled threaded process
that is spinning as using 100% cpu.
This is generally more useful than the current 'Solaris mode' that
reports 100/num_cpu% cpu.

A multi-threaded program that is busy on multiple cpu will
show a cpu use > 100%.

Enabled by -I on the command line or 'i' on stdin.

Remove some (uint16_t) casts from the 'delta jiffies' value.
With enough cpu, fast enough HZ, and long enough delay I think
64k can be exceeded.
I doubt intended efficiency of 16x16 multiple makes any difference.

Signed-off-by: David Laight 
---

It might be worth adding an extra column for processes with a
lot of busy threads on systems with a lot of cpu.

Note that this is a normal diff from an installed tree.
I don't have the git tree.

--- top.c.1.33  2021-11-11 09:49:08.165982660 +
+++ top.c   2021-11-11 11:28:50.155674293 +
@@ -166,6 +166,7 @@
 #endif
 #if ENABLE_FEATURE_TOP_SMP_CPU
smallint smp_cpu_info; /* one/many cpu info lines? */
+   smallint irix_mode;/* 100% => one cpu busy */
 #endif
unsigned lines;  /* screen height */
 #if ENABLE_FEATURE_TOP_INTERACTIVE
@@ -202,6 +203,7 @@
 #define sort_field   (G.sort_field)
 #define inverted (G.inverted  )
 #define smp_cpu_info (G.smp_cpu_info  )
+#define irix_mode(G.irix_mode )
 #define initial_settings (G.initial_settings  )
 #define sort_function(G.sort_function )
 #define prev_hist(G.prev_hist )
@@ -223,8 +225,9 @@
OPT_n = (1 << 1),
OPT_b = (1 << 2),
OPT_H = (1 << 3),
-   OPT_m = (1 << 4),
-   OPT_EOF = (1 << 5), /* pseudo: "we saw EOF in stdin" */
+   OPT_I = (1 << 4),
+   OPT_m = (1 << 5),
+   OPT_EOF = (1 << 6), /* pseudo: "we saw EOF in stdin" */
 };
 #define OPT_BATCH_MODE (option_mask32 & OPT_b)
 
@@ -314,6 +317,13 @@
return;
 #else
if (!smp_cpu_info) {
+   if (irix_mode && !num_cpus) {
+   jiffy_counts_t dummy_jif;
+   while (read_cpu_jiffy(fp, &dummy_jif) >= 4)
+   num_cpus++;
+   if (!num_cpus)
+   irix_mode = 0;
+   }
fclose(fp);
return;
}
@@ -328,8 +338,10 @@
break;
num_cpus++;
}
-   if (num_cpus == 0) /* /proc/stat with only "cpu ..." line?! */
+   if (num_cpus == 0) { /* /proc/stat with only "cpu ..." line?! */
smp_cpu_info = 0;
+   irix_mode = 0;
+   }
 
cpu_prev_jif = xzalloc(sizeof(cpu_prev_jif[0]) * num_cpus);
 
@@ -602,6 +614,28 @@
return meminfo[MI_MEMTOTAL];
 }
 
+#if ENABLE_FEATURE_TOP_DECIMALS
+# define UPSCALE 1000
+# define FMT "%s"
+# define SHOW_STAT(name, buf_id) fmt_proc_pc(name, buf_id)
+static NOINLINE const char *fmt_proc_pc(unsigned value, unsigned buf_id)
+{
+   static char bufs[2][16];
+   char *buf = bufs[buf_id];
+
+   if (value >= 1000)
+   snprintf(buf, 16, "%5u", value/10);
+   else
+   snprintf(buf, 16, "%3u.%c", value/10, '0' + (value % 10));
+   return buf;
+}
+
+#else
+# define UPSCALE 100
+# define FMT "%4u%%"
+# define SHOW_STAT(name, buf_id) name
+#endif
+
 static NOINLINE void display_process_list(int lines_rem, int scr_width)
 {
enum {
@@ -627,17 +661,6 @@
" COMMAND");
lines_rem--;
 
-#if ENABLE_FEATURE_TOP_DECIMALS
-# define UPSCALE 1000
-# define CALC_STAT(name, val) div_t name = div((val), 10)
-# define SHOW_STAT(name) name.quot, '0'+name.rem
-# define FMT "%3u.%c"
-#else
-# define UPSCALE 100
-# define CALC_STAT(name, val) unsigned name = (val)
-# define SHOW_STAT(name) name
-# define FMT "%4u%%"
-#endif
/*
 * %VSZ = s->vsz/MemTotal
 */
@@ -664,14 +687,18 @@
 * we assume that unsigned is at least 32-bit.
 */
pcpu_shift = 6;
-   pcpu_scale = UPSCALE*64 * (uint16_t)busy_jifs;
+   pcpu_scale = UPSCALE*64 * busy_jifs;
if (pcpu_scale == 0)
pcpu_scale = 1;
while (pcpu_scale < (1U << (BITS_PER_INT-2))) {
pcpu_scale *= 4;
pcpu_shift += 2;
}
-   tmp_unsigned = (uint16_t)(cur_jif.total - prev_jif.total) * total_pcpu;
+   tmp_unsigned = (cur_jif.total - prev_jif.total) * total_pcpu;
+#if ENABLE_FEATURE_TOP_SMP_CPU
+   if (irix_mode && num_cpus)
+   tmp_unsigned /= num_cpus;
+#endif
if (tmp_unsigned != 0)
pcpu_sca