Re: [PATCH] Add busybox

[Justin Cinkelj]

On 08/20/2017 05:15 PM, Nadav Har'El wrote:




On Fri, Aug 18, 2017 at 3:17 PM, Justin Cinkelj 
> wrote:


Thank you for commit, and for suggested improvements.

3 inter-dependent patches were sent, 2 for osv, 1 for osv-apps
(that one includes 2 patches for busybox source).

I wasn't sure if osv_execve should (is allowed to) clear errno on
success. This doesn't make any sense in linux - new program will
start executing, and nobody can check the errno in old program.
But for busybox/ash, someone (ash or OSv) has to clear it, so I
moved that to OSv.

If I understand correctly, syscalls are permitted to clear errno
on success (but most of them don't), and errno value should be
retrieved immediately after failed call. So setting errno to 0 is
not illegal.

Even with these patches, ash is still not 'fully functional'. I
see (only now) that script can be run as
run.py -e 'ash -c /cmd0.sh'
but not as:
run.py -e 'ash'
/ # /cmd0.sh
ash: /cmd0.sh: not found

Second case fails, as osv::app tries to execute cmd0.sh, which is
not a shared object file. I guess '!#/usr/bin/ash' should be added
to head of cmd0.sh, and ash should check for it.


In Linux, and Unix starting the mid 1980s, execve() itself checks the 
beginning of the file for the "shebang" characters: "#!", and if it 
exists, it runs that command specified there instead of the file the 
user asked to run.


If the Shebang is *missing* in the file, then Linux assumes it is a 
native Linux executable. But if it turns out not to be, execve() 
returns ENOEXEC. When ash or other shells get ENOEXEC from their 
execve() call, they assume (or should assume, I didn't check the ash 
source code) that this is a shell script, and run it themselves.

Thank you for explanation; I'm glad I mentioned the rather obvious problem.


So I think there are two things to fix - the first one probably more 
interesting and the second can be postponed indefinitely:


1. osv_execve() should recognize the case where the files it is trying 
to load doesn't have the executable magic header, and fail with 
ENOEXEC. This will hopefully allow ash to run scripts without a 
shebang. I don't remember what the code in core/elf.cc does in this 
case, if it doesn't fail gracefully when running a non-executable, it 
should be fixed. Please let me know if you need help with that. I'll 
open an issue.
I looked at ash, and there is if(errno==ENOEXEC) in ash.c code (tryexec 
function), and - 20 line long comment there also explains what you 
already said. So this approach should work.


2. osv_execve() should recognize the shebang in a file and behave 
Linux. This will allow running scripts with shebangs as expected.


For the same reason, cmd0.sh cannot call a second script.

I'm also suspicious about what would happen when called script
does exit or return. Maybe also a parent shell(s) will exit/return.


Normally, a script is run in a separate process - in Unix, when 
shebang is not used, this is a "subshell" (a *forked* copy of the 
original shell), when shebang is used this is a completely new shell 
(fork and exec). Since it is a separate process, it can exit normally.


Since you modified ash to replace fork by a new thread, and "exit" 
with exiting a thread, maybe all of this would just work normally.
However, what really worries me is that unlike Unix processes, when an 
OSv thread exits, nothing is cleaned behind it: memory allocated 
remain allocated, open files remain open, etc. All these subshells may 
be leaking resources. It might be good enough for one-of things, but 
may cause problems if you intend to run a lot of these shells on the 
same VM - although I doubt anyone would.
Yes, I realized that. Hopefully busybox valgrind/memory tests helped to 
remove many of the leaks. One hard limit is the max 32 ELF namespaces 
(which are never reclaimed); so we can run ls only 32-times. I guess 
interactive use will hit that pretty fast :/


What I hope for is that Miha will use environment variables expansion, 
if/while sentences and other 'light' features of ash. Light features - I 
think some of busybox modules are implemented in a way that permit to 
just call their main, without doing execve/osv_execve; but maybe I 
couldn't use that, because I needed to build busybox as a shared lib, 
and both options might be mutually exclusive.


I noticed ENABLE_FEATURE_CLEAN_UP somewhere in busybox code. I'm not 
sure what it is, but it might be something we want to have enabled.


Justin



Anyway, ash will be now a bit more useful, and Miha can start
writing ash scripts. But only simple scripts - calling an
additional script is considered an advanced topic, and he should
not try that ;)

Justin




--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from 

Re: [PATCH] Add busybox

[Nadav Har'El]

On Sun, Aug 20, 2017 at 6:15 PM, Nadav Har'El  wrote:

>
>
> 1. osv_execve() should recognize the case where the files it is trying to
> load doesn't have the executable magic header, and fail with ENOEXEC. This
> will hopefully allow ash to run scripts without a shebang. I don't remember
> what the code in core/elf.cc does in this case, if it doesn't fail
> gracefully when running a non-executable, it should be fixed. Please let me
> know if you need help with that. I'll open an issue.
>

I opened an issue on this:

https://github.com/cloudius-systems/osv/issues/904

Should be fairly easy to fix, I think.

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] Add busybox

[Justin Cinkelj]

I mentioned BSD licence in Makefile only. I can remove it if that makes 
patch busybox/GPL compliant, and is also acceptable for OSv.


--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] Add busybox

[Benoît Canet]

oh this is nice.

Are we sure of busybox license ?

2017-08-07 11:32 GMT+02:00 Justin Cinkelj :

> Busybox ash shell is more powerful than OSv --runscript option.
> Use ash when fancy shell is required.
>
> While busybox config file includes default options (e.g. nearly all
> options are enabled), the usr.manifiest doesn't try to create all
> valid symlinks to busybox binary. Instead, only a few are included.
>
> A small test script is provided (commented out in usr.manifest).
> It can be run as:
> ./scripts/run.py -e "--env=PATH=/usr/lib sleep 1.2; ash /cmd0.sh; echo
> Done"
>
> Signed-off-by: Justin Cinkelj 
> ---
>  busybox/.gitignore |3 +
>  busybox/GET|   40 +
>  busybox/Makefile   |   13 +
>  busybox/patches/0001-OSv-remove-exit-usage.patch   |  190 
>  ...lude-file-line-function-into-TRACE-output.patch |   55 +
>  .../0003-OSv-ash-fix-process-spawning.patch|  263 +
>  busybox/patches/busybox-main.c |5 +
>  busybox/patches/config | 1139
> 
>  busybox/test-script/cmd0.sh|   26 +
>  9 files changed, 1734 insertions(+)
>  create mode 100644 busybox/.gitignore
>  create mode 100755 busybox/GET
>  create mode 100644 busybox/Makefile
>  create mode 100644 busybox/patches/0001-OSv-remove-exit-usage.patch
>  create mode 100644 busybox/patches/0002-ash-include-file-line-function-
> into-TRACE-output.patch
>  create mode 100644 busybox/patches/0003-OSv-ash-
> fix-process-spawning.patch
>  create mode 100644 busybox/patches/busybox-main.c
>  create mode 100644 busybox/patches/config
>  create mode 100644 busybox/test-script/cmd0.sh
>
> diff --git a/busybox/.gitignore b/busybox/.gitignore
> new file mode 100644
> index 000..d0721e6
> --- /dev/null
> +++ b/busybox/.gitignore
> @@ -0,0 +1,3 @@
> +build/
> +usr.manifest
> +
> diff --git a/busybox/GET b/busybox/GET
> new file mode 100755
> index 000..90f2ab1
> --- /dev/null
> +++ b/busybox/GET
> @@ -0,0 +1,40 @@
> +#!/bin/bash
> +
> +set -e
> +
> +URL=https://github.com/mirror/busybox
> +VERSION_GIT=1_27_stable
> +VERSION_LIB=1.27.1
> +
> +[ ! -d build ] && mkdir build
> +cd build
> +if [ ! -d busybox ]
> +then
> +  git clone --branch $VERSION_GIT --depth 1 $URL
> +  (cd busybox && git am ../../patches/000[0-9]-*\.patch)
> +fi
> +cd busybox
> +
> +cp -f ../../patches/config .config
> +make -j4
> +(cd 0_lib/ && ln -sf libbusybox.so.${VERSION_LIB} libbusybox.so)
> +gcc -o busybox-main.so ../../patches/busybox-main.c -L0_lib -lbusybox
> -shared -fPIC
> +cd ../..
> +
> +cat < usr.manifest
> +/usr/lib/libbusybox.so.${VERSION_LIB}: \${MODULE_DIR}/build/busybox/
> 0_lib/libbusybox.so.${VERSION_LIB}_unstripped
> +/usr/lib/busybox: \${MODULE_DIR}/build/busybox/busybox-main.so
> +/usr/lib/ash: ->/usr/lib/busybox
> +#
> +# common utilities
> +/usr/lib/df: ->/usr/lib/busybox
> +/usr/lib/sleep: ->/usr/lib/busybox
> +/usr/lib/ls: ->/usr/lib/busybox
> +/usr/lib/cat: ->/usr/lib/busybox
> +/usr/lib/echo: ->/usr/lib/busybox
> +/usr/lib/ping: ->/usr/lib/busybox
> +#
> +# test scripts
> +## /**: \${MODULE_DIR}/test-script/**
> +
> +EOF
> \ No newline at end of file
> diff --git a/busybox/Makefile b/busybox/Makefile
> new file mode 100644
> index 000..5b56cb1
> --- /dev/null
> +++ b/busybox/Makefile
> @@ -0,0 +1,13 @@
> +#
> +# Copyright (C) 2017 XLAB d.o.o.
> +#
> +# This work is open source software, licensed under the terms of the
> +# BSD license as described in the LICENSE file in the top-level directory.
> +#
> +
> +.PHONY: module
> +module:
> +   ./GET
> +
> +clean:
> +   rm -rf build
> diff --git a/busybox/patches/0001-OSv-remove-exit-usage.patch
> b/busybox/patches/0001-OSv-remove-exit-usage.patch
> new file mode 100644
> index 000..515e510
> --- /dev/null
> +++ b/busybox/patches/0001-OSv-remove-exit-usage.patch
> @@ -0,0 +1,190 @@
> +From 12453fd712c7874609eb4bcb2246a37c00571161 Mon Sep 17 00:00:00 2001
> +From: Justin Cinkelj 
> +Date: Thu, 3 Aug 2017 09:53:51 +0200
> +Subject: [PATCH 1/3] OSv: remove exit() usage
> +
> +exit() would shutdown whole OSv VM, remove it.
> +This requires removing noreturn attribute from related functions.
> +
> +Signed-off-by: Justin Cinkelj 
> +---
> + include/libbb.h|  6 +++---
> + libbb/appletlib.c  | 28 ++--
> + libbb/verror_msg.c |  2 +-
> + libbb/xfunc_die.c  |  4 ++--
> + 4 files changed, 20 insertions(+), 20 deletions(-)
> +
> +diff --git a/include/libbb.h b/include/libbb.h
> +index 8eccd81..bef7437 100644
> +--- a/include/libbb.h
>  b/include/libbb.h
> +@@ -1108,7 +1108,7 @@ int spawn_and_wait(char **argv) FAST_FUNC;
> + int run_nofork_applet(int applet_no, char **argv) FAST_FUNC;
> + #ifndef BUILD_INDIVIDUAL
> + extern int find_applet_by_name(const char *name) FAST_FUNC;