Re: [git commit] hostid: do not output sign-extended host id. Closes 6056

[Ralf Friedl]

Denys Vlasenko wrote:

On Wednesday 06 March 2013 08:27, Thierry Reding wrote:
  

On Mon, Mar 04, 2013 at 03:04:38AM +0100, Denys Vlasenko wrote:


commit: 
http://git.busybox.net/busybox/commit/?id=9bbf6b98c42a212b8a4b1aa02975ac18bb612922
branch: http://git.busybox.net/busybox/commit/?id=refs/heads/master

Signed-off-by: Denys Vlasenko 
---
 coreutils/hostid.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/coreutils/hostid.c b/coreutils/hostid.c
index 5c1a4e0..e5b1f51 100644
--- a/coreutils/hostid.c
+++ b/coreutils/hostid.c
@@ -36,7 +36,8 @@ int hostid_main(int argc UNUSED_PARAM, char **argv 
UNUSED_PARAM)
bb_show_usage();
}
 
-	printf("%08lx\n", gethostid());

+   /* POSIX says gethostid returns a "32-bit identifier" */
+   printf("%08x\n", (unsigned)(uint32_t)gethostid());
  

That's a bit over the top. uint32_t is already unsigned.



But unsigned may one day be wider than 32 bits,
therefore, formally, if you want to print uint32_t using %d,
%u or %x, you need to widen it.
Actually, you could pass unsigned char to printf and the compiler would 
widen it automatically.

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

Re: ash: "source " makes ash exit?

[Ralf Friedl]

Cathey, Jim wrote:

So if I should type, manually, ". oopsImisspelledIt"
my session is supposed to go away?
  

No, only an non-interactive shell is supposed to exit.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: CTRL-ALT-DEL not working as expected. Seeking for advices

[Ralf Friedl]

Laurent Bercot wrote:

kernel (hd0,0)/vmlinuz-... root=/dev/sda2 init=/root/a.out

  Depending on your kernel, it may be a little more complicated than that.
If your kernel has been compiled with an initramfs, the first userland
process won't be /root/a.out, but the /init file in the initramfs.
The kernel is never compiled with an initramfs. The kernel is often 
compiled to use an initramfs, if the boot loader supplies one. If you 
don't load an initramfs (with the command initrd in grub), the kernel 
won't use one.
The other question is whether the kernel has all necessary drivers 
compiled in to mount the root filesystem. But if the kernel mounts the 
root partition and starts the program when you don't pass an initrd 
parameter, you can be sure that this /root/a.out was started directly by 
the kernel.

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

Re: [PATCH] sulogin: allow system maintenance login if root password is empty

[Ralf Friedl]

Jonathan Liu wrote:

The current password checking is unable to distinguish between the user
entering an empty password or pressing Control-D. As a result, an empty
password always results in normal startup.

We modify bb_ask to store the EOF status after the null terminator if
the password is empty. This allows sulogin to properly check if
Control-D was pressed.
  

What about returning NULL when EOF is pressed?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Cross-compilation

[Ralf Friedl]

Laurent Bercot wrote:

Autotools-based can be good or bad for cross-compiling. The biggest
issue is that lots of people write broken tests that need to run test
programs to get the results they want.

  As long as there are differences between systems, build-time tests
will be necessary to check for what the system provides and what
needs working around.
  This is the main problem with cross-compilation: build-time tests
only work in the native case. When cross-compiling, there is no way to
automatically figure out what the target flags are.

  The best solution would be to have a centralized, worldwide database
of system-dependent flags and options that any cross-compilation process
could search and extract target information from. Until we have that,
the only clean way to cross-compile is to have the user provide target
sysdeps *by hand*.

  As far as I am aware, autotools do not make that easy. This essential
part of the cross-compilation process is not integrated into the
autotools architecture, which is why I do not consider them a reliable
system.

Actually it is very easy:
You just need a file that contains the answers to the questions that 
configure can't figure out itself. Then you either export all these 
values before running configure, or you export CONFIG_SITE=/path/to/file 
and configure will read the file.

The file contains lines like
ac_cv_epoll_works=yes
ac_cv_c_littleendian=yes
ac_cv_c_bigendian=no
ac_cv_type_pid_t=yes
ac_cv_type_ptrdiff_t=yes
ac_cv_type_struct_sockaddr_storage=yes
ac_cv_sizeof_char=1
ac_cv_sizeof_short=2
ac_cv_sizeof_int=4
ac_cv_sizeof_long=4
ac_cv_sizeof_long_long=8
ac_cv_sizeof_void_p=4
ac_cv_sizeof_pid_t=4
ac_cv_sizeof_ptrdiff_t=4
ac_cv_sizeof_size_t=4
ac_cv_sizeof_ssize_t=4
ac_cv_sizeof_socklen_t=4

configure will then not run the tests, but use these answers you 
provided. As this worldwide database you mention is also created *by 
hand*, just by someone else, I consider that essentially the same. 
Depending on the target, it may be possible to run the configure scripts 
on the target to find out the correct answers, although it may not be 
feasible to run the whole compilation on the target.
Note that some of the entries above can and will be detected 
automatically by configure just by calling the cross compiler without 
running the actual binary.


That covers the tests that come with the autotools. It may be 
problematic when people add other tests that don't follow these rules, 
but that can't be blamed on autotools.


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

Re: [PATCH] ubimkvol: add -m option to create volume of maximum size

[Ralf Friedl]

walter harms schrieb:


Am 03.06.2013 20:45, schrieb Paul B. Henson:

On 6/3/2013 3:48 AM, walter harms wrote:


+p = path + sprintf(path, "/sys/class/ubi/ubi%d/", num);
+
+strcpy(p, "avail_eraseblocks");

you can use xasprintf() here ?

I dunno, Mike said to use sprintf/strcpy to avoid copying the same
string prefix twice.

What would be the benefit of allocating the space for the string off of
the heap instead of the stack?


I do not see the whole code but so far i understand you
like to construct one string with some additional stuff.

IMHO it is most times more easy and less error-prone to
construct the strings with asprintf() instead of adding as
pieces together by hand. This is clearly no true for every
problem. Please think about the problem it is ok when you
conclude that it is not working.
In this case the buffer is alread in a static variable, so just use one 
sprintf for everything, no need to append a constant string with strcpy.


sprintf(path, "/sys/class/ubi/ubi%d/avail_eraseblocks", num);

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

Re: [PATCH] udhcpc6 source address is null, should be link-local address

[Ralf Friedl]

Denys Vlasenko wrote:

But what if interface has no IPv6 addresses?
Isn't the whole purpose of DHCP is to *acquire an address*?

For example, I don't use or need IPv6 for now.
I just checked, and as expected, none of my interfaces have
any IPv6 addresses.

What should happen in this case?
Does an iface absolutely need a link-local address (and how
to acquire one?).
If not, what dhcp client should fill in - all zeros?
If yes, why can't it just always do that?

From RFC 4862:

link-local address -  an address having link-only scope that can be used to 
reach neighboring nodes attached to the same link.  All interfaces have a 
link-local unicast address.


From RFC 4291:

  Links or Nodes with IEEE 802 48-bit MACs

EUI64  defines a method to create an IEEE EUI-64 identifier from an
   IEEE 48-bit MAC identifier.  This is to insert two octets, with
   hexadecimal values of 0xFF and 0xFE (see the Note at the end of
   appendix), in the middle of the 48-bit MAC (between the company_id
   and vendor-supplied id).  An example is the 48-bit IEEE MAC with
   Global scope:

   |0  1|1  3|3  4|
   |0  5|6  1|2  7|
   ++++
   |cc0g|||
   ++++


   where "c" is the bits of the assigned company_id, "0" is the value of
   the universal/local bit to indicate Global scope, "g" is
   individual/group bit, and "m" is the bits of the manufacturer-
   selected extension identifier.  The interface identifier would be of
   the form:

   |0  1|1  3|3  4|4  6|
   |0  5|6  1|2  7|8  3|
   +++++
   |cc1g||1110||
   +++++

   When IEEE 802 48-bit MAC addresses are available (on an interface or
   a node), an implementation may use them to create interface
   identifiers due to their availability and uniqueness properties.


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

Re: [PATCH] guess_fstype applet

[Ralf Friedl]

James B wrote:

klibc has "fstype" binary to detect filesystem types during boot time.
util-linux mount has "mount -guess-fstype" to do the same.
Why not use printf? It's already used in busybox, so it wont be included 
just for this applet. I'm sure it would also make the code shorter. (I 
do hope that the compiler can optimize out the calls to strlen for the 
constant strings.)


How about this:
char const *type;
if ((!volume_id_probe_all (id, 0)) && id->type)
  type = id->type;
else {
  type = "unknown";
  retcode = 1;
}
if (argc > 2)
  printf ("%s: ", argv[0]);
printf ("%s\n", type);
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

set_loop tries to create 1048575 loop devices

[Ralf Friedl]

Hi

I just had mount create a large number of loop devices before I 
interrupted it.
The reason is that the loop module is not loaded in the kernel, but it 
would be better to have an error message instead.


Here ist the strace output:
stat64("/dev/loop0", 0x7ffd8840)= -1 ENOENT (No such file or 
directory)

mknod("/dev/loop0", S_IFBLK|0644, makedev(7, 0)) = 0
open("/dev/loop0", O_RDONLY|O_LARGEFILE) = -1 ENXIO (No such device or 
address)
stat64("/dev/loop1", 0x7ffd8840)= -1 ENOENT (No such file or 
directory)

mknod("/dev/loop1", S_IFBLK|0644, makedev(7, 1)) = 0
open("/dev/loop1", O_RDONLY|O_LARGEFILE) = -1 ENXIO (No such device or 
address)

...
stat64("/dev/loop255", 0x7ffd8840)  = -1 ENOENT (No such file or 
directory)

mknod("/dev/loop255", S_IFBLK|0644, makedev(7, 255)) = 0
open("/dev/loop255", O_RDONLY|O_LARGEFILE) = -1 ENXIO (No such device or 
address)
stat64("/dev/loop256", 0x7ffd8840)  = -1 ENOENT (No such file or 
directory)

mknod("/dev/loop256", S_IFBLK|0644, makedev(7, 0)) = 0
open("/dev/loop256", O_RDONLY|O_LARGEFILE) = -1 ENXIO (No such device or 
address)

...

I don't know whether there are other reasons why open /dev/loop might 
return ENXIO, but if not, then set_loop should abort when it receives 
that error.


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

Re: set_loop tries to create 1048575 loop devices

[Ralf Friedl]

Lauri Kasanen wrote:

On Mon, Jan 20, 2014, at 18:52, Ralf Friedl wrote:

Hi

I just had mount create a large number of loop devices before I
interrupted it.
The reason is that the loop module is not loaded in the kernel, but it
would be better to have an error message instead.
...

I don't know whether there are other reasons why open /dev/loop might
return ENXIO, but if not, then set_loop should abort when it receives
that error.

According to libbb/loop.c, it also returns ENXIO to mean "this loop is
free". So can't exactly abort for that.

I recommend you disable ENABLE_FEATURE_MOUNT_LOOP_CREATE if you don't
want the devices created.
It's not that I don't want the loop functionality, it's just that the 
loop module was not loaded and trying to open /dev/loop0 didn't trigger 
the module to be loaded.


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

Re: set_loop tries to create 1048575 loop devices

[Ralf Friedl]

Denys Vlasenko schrieb:

Yes, good idea.

Please try current git, I pushed a fix there.

Works very well, thank you.

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

Re: [PATCH] correct_password: Handle NULL from crypt

[Ralf Friedl]

Lauri Kasanen wrote:

As with many other software, busybox was also broken by the glibc >=
2.17 behavior change. Now crypt() returns NULL if either salt or
password is invalid.

This causes busybox 1.21, 1.22, and git su to segfault, when you just
press enter at the password prompt (configured to use system crypt() of
course).

The attached patch fixes su. You may want to check every other call to
crypt() in busybox.

- Lauri
A simple way to fix this for all users of pw_encrypt is to change 
pw_encrypt:

char* FAST_FUNC pw_encrypt(const char *clear, const char *salt, int cleanup)
{
char *res = crypt(clear, salt);
if (!res)
res = ""; // Or whatever value crypt previously returned
return xstrdup(res);
}

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

Re: Ntpd config file support

[Ralf Friedl]

Hi Laszlo

First, please either write your message below the quotes, or omit the 
quotes. Especially don't quote parts that are not relevant to your message.


Laszlo Papp wrote:

At least three people expressed that it is about convenience, a useful
one.
Well, all of them didn't provide a convincing argument. And most others 
on this list wonder why you make such a big deal about something that 
can be solved with a few lines in a shell script. If you want a 
configuration file only for the time servers, this script will give you 
compatibility to the ntp.org config file:

#!/bin/sh
NTPD_OPTIONS="..."
exec busybox ntpd $NTPD_OPTIONS $(sed -nre 's/^server *(.*)$/-p 
\1/g'/etc/ntp.conf )



Anyway, please comment on the init script that I posted earlier when
you get around to it, as well as the corresponding config file..

I'm not Harald, but I will comment on it anyway.
Short version:
It's a very nice script. Use it. Be very happy with it.

Long version:
I guess it fits into your distibution, but it's useless for most others. 
That was one of the points Harald made, you want things specific to your 
distibution in Busybox, where they don't belong. You could place it 
somewhere in the contrib directory, but it would probably be a waste of 
space and bandwidth.
Most distributions come with a template for such a script, and it 
shouldn't take more than a few minutes to adapt such a template.
You use start-stop-daemon. Why do you think everybody would even have 
the program, or want to use it?
Your script has a reaload case, where you send SIGHUP. What should ntpd 
do on SIGHUP? Reload the config file? You said that reloading the config 
is not necessary when it was about code size, so why send SIGHUP?

Finally, the export in the configuration file is not necessary.

In one email in this thread someone suggested to make the configuration 
hardcoded as the compile time configuration. I just hope that was meant 
as a joke.



> Usually scripts in/etc/init.d use /etc/default/* as config values
> (some distros, even using them as main config files). The scripts that
> Laszlo posted fit that pattern.
Not quite; actually "/etc/default" is more like a Debian, et al,
pattern. OpenWrt will use something. Yocto uses something else, etc.
And for what it is worth, buildroot is also strange with
"/etc/default/ntpd" without any busybox indication.
Which again shows that distributions are different, so which distibution 
style should Busybox pick? It is up to the distribution to provide the 
right information to the program.



I would ask this question from myself if I were you: what do I gain or
lose with such a feature added or rejected? ...
That is a good question if maximizing the number of users is the top 
priority.


Also someone suggested that devices today have GHz CPUs and at least 
512MB RAM. Well, maybe they are not the target audience of busybox. 
There are also devices with 16MB RAM and 4MB flash.



I do not follow. The busybox user is the distro and maintainer in this
case, really. The end user is not necessarily even aware of busybox,
I would really appreciate more respect here towards end users.
The end users have raised their opinion how they
would like to see your software behaving.
I hope you realize that you are contradicting yourself here, within a 
few lines of a single email.

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

Re: Ntpd config file support

[Ralf Friedl]

Laszlo Papp wrote:

On Tue, Mar 18, 2014 at 9:17 PM, Ralf Friedl  wrote:

First, please either write your message below the quotes, or omit the
quotes. Especially don't quote parts that are not relevant to your message.

I have no idea what point you are trying to make in here, sorry, nor
do I care about the details 'cause the thread is already huge.

I should have guessed so.


If you want a configuration file
only for the time servers, this script will give you compatibility to the
ntp.org config file:
#!/bin/sh
NTPD_OPTIONS="..."
exec busybox ntpd $NTPD_OPTIONS $(sed -nre 's/^server *(.*)$/-p
\1/g'/etc/ntp.conf )

I wonder if you are serious about this compared to the alternative way:

/etc/busybox-ntpd/busybox-ntpd.conf
foo=bar

I guess you are trying to be smart in here without actually realizing
significant difference between the two versions. I would not even
bother to use busybox's ntpd if I had to write such ugly lines that is
much more difficult to maintain than needed to.
I have no preference either way, and there are advantages and drawbacks 
to both approaches. I didn't say that there are no differences, so no 
need for you to imply that such a statement would have been wrong.

Seriously though, you did not get the point of the thread, have you?
*No one* has suggested to add initscripts to busybox. Please do
re-read the thread. A short configuration file was just suggested. You
are the first one indicating that initscript would need to be added.
Well, someone using your email adresse has posted an init script and 
asked for opinions about it. And no, I won't re-read the thread, it 
would be more waste of time.
Anybody who seriously considers making a distribution should be able to 
put such a script tugether within a few minutes without asking here for 
help.



Your script has a reaload case, where you send SIGHUP. What should ntpd do
on SIGHUP? Reload the config file? You said that reloading the config is not
necessary when it was about code size, so why send SIGHUP?

Sorry, I cannot follow your logic in here. There is no any need for
signal handling in order to load a config on start, really.
You can't follow, or you don't want to. It is about these lines in the 
init script someone proposed in your name:


+reload)
+echo -n "reloading $DESC: $NAME... "
+killall -HUP $(basename ${DAEMON})



In one email in this thread someone suggested to make the configuration
hardcoded as the compile time configuration. I just hope that was meant as a
joke.

Not at all, no. It was serious, at least from my side.
It thought it was from someone else, but if you are serious about it, 
just do it.

I do not follow. The busybox user is the distro and maintainer in this
case, really. The end user is not necessarily even aware of busybox,
I would really appreciate more respect here towards end users.
The end users have raised their opinion how they
would like to see your software behaving.

I hope you realize that you are contradicting yourself here, within a few
lines of a single email.

No, I do not. I still stick by my opinion in here I am afraid.
I will try to explain it slowly. In the first sentence you say the end 
user is not even aware of busybox. Then you say that end users have here 
raised their opinion about config files and ntpd.

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

Re: Ntpd config file support

[Ralf Friedl]

Mike Dean wrote:
You mentioned earlier that code is never freed from memory.  If you're 
this concerned with bloat, why don't you fix your problem of never 
deallocating the init code instead of worrying about some small 
feature like this?  The Linux kernel does this; that's why you see the 
message about "Freeing xxKB of memory" late in the boot process.  The 
freed memory is the memory that was used by the initialization code. 
 This trick isn't limited to kernelspace.  You can dynamically link 
libraries with init code, and you can unlink them at runtime as well. 
 If you really want to reduce bloat, you can take a page from the 
kernel devs and free all that init code after the binary is up and 
running.  If you really are concerned with bloat, that is.
One important difference is that the kernel is loaded once and then runs 
for a long time, for different values of long.


Busybox implements a large number of programs, so in most systems it is 
likely that at least one of those will be executed regularly. Therefor, 
those pages you call init will have to be loaded again and again, so 
there would be no advantage in memory usage.
But you will have the disadvantages of the approach. It's more difficult 
to read and to maintain, as you have to write something like dlopen, 
dlsym, call init, dlclose instead of just init. For the same reason it 
also uses more memory, four function calls instead of one. The code in 
shared libraries tends to be larger because it has to be position 
independent. In addition, this would also increase the storage space 
required. The text and data segments are aligned to the page size. The 
shared library has it's own program header, relocation entries, names of 
the exported symbols. There are embedded devices running on 4MB flash 
that use busybox. Besides the work required for such an approach, the 
result would be actually worse.
You would also either require a dynamic linker to be present on the 
target machine, and the target to support dynamic libraries and dlopen. 
Or you would have to maintain two versions of each call to an init function.


What might be useful is labeling the functions and data structures as 
init_text and init_data, for the case where a device uses mostly long 
running processes. With help from the linker, these segments could be 
arranged together, and the kernel would just drop them when they are not 
used and memory is needed. It would not increase memory usage or storage 
requirements, and it doesn't have to be done all at once. And for 
targets that don't support it, it could just be left out, by defining 
the used macros as empty.

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

Re: [PATCH] Ntpd config file support

[Ralf Friedl]

Harald Becker wrote:

Your program will fail on lines starting with the word server
(eg. serverxyz), that is it does not check for clear word
boundary and gives wrong results in that case.
The program will not fail for serverxyz, it will add a server "xyz". 
This may be a bug or a feature :-)



while (cbuf[i] > 35) i++;

Unwise to do this in a not poor ASCII environment, as most
systems are nowadays. This way you allow unprintable and any
kind of illegal characters in time server addresses.

What is special about 35? Why is ' fine while " is not?

Of course the configuration comes from someone who already has root 
access, so whatever happens here to an invalid input can't be worse than 
"rm -rf /".

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

Re: /proc//cmdline and udhcpcd

[Ralf Friedl]

Matt Whitlock wrote:

On Tuesday, 25 March 2014, at 10:52 pm, Cristian Ionescu-Idbohrn wrote:

I find out that using that option:

-x hostname:foo
   ^

shows up in /proc//cmdline as:

-x hostname foo
   ^

/proc//cmdline reflects any changes that the process has made to its argv array. 
It's common when parsing a command line for a program to replace delimiters with null 
bytes. The /proc//cmdline interface converts null bytes to spaces for ease of 
display and parsing.

My guess is also that udhcpcd does something like
strchr(hostname, ':') = '\0';

But the /proc//cmdline interface doesn't convert null bytes to 
spaces. Doing so would lose information. Some programs may choose to do 
this conversion, but programs reading cmdline must be prepared for null 
bytes anyway.


hex -g /proc/30883/cmdline
 2f 75 73 72 2f 73 62 69 6e 2f 61 63 70 69 64 00 /usr/sbin/acpid.
0010 2d 6e 00 2d 66 00 -n.-f.

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

Re: dc hitting a compiler bug, or undefined behavior

[Ralf Friedl]

Lauri Kasanen wrote:

I'm seeing busybox dc acting funny when compiled with some versions of
gcc. This is on busybox git. The x86 binary busybox_unstripped and
config are attached.

gcc 4.2.2 - ok
gcc 4.7.2:
nc 10 1 add p
2.738e+93

So either bb is hitting a compiler bug, or undefined behavior somewhere
with new gcc's more aggressive optimizations.
I have 4.7.2 on x86 and I get 11 as output. You could add debug output 
to push and pop to see what happens.


Whether the output 11 is to be expected is another question:
$ dc --version
dc (GNU bc 1.06.95) 1.3.95
$ dc --help
Usage: dc [OPTION] [file ...]
  -e, --expression=EXPRevaluate expression
  -f, --file=FILE  evaluate contents of file
  -h, --help   display this help and exit
  -V, --versionoutput version information and exit
$ dc 10 1 add p
dc: Could not open file 10
dc: Could not open file 1
dc: Could not open file add
dc: Could not open file p
$ dc -e '10 1 add p'

$ dc -e '10 1 + p'
11

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

Re: dc hitting a compiler bug, or undefined behavior

[Ralf Friedl]

Lauri Kasanen schrieb:

On Sun, Mar 30, 2014, at 15:05, Ralf Friedl wrote:

I'm seeing busybox dc acting funny when compiled with some versions of
gcc. This is on busybox git. The x86 binary busybox_unstripped and
config are attached.

gcc 4.2.2 - ok
gcc 4.7.2:
nc 10 1 add p
2.738e+93

So either bb is hitting a compiler bug, or undefined behavior somewhere
with new gcc's more aggressive optimizations.

I have 4.7.2 on x86 and I get 11 as output. You could add debug output
to push and pop to see what happens.

I think it's related to
http://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
which hit the radeon driver as well
http://lists.freedesktop.org/archives/dri-devel/2013-August/044122.html

but changing the stack[1] to stack[0], stack[], or even stack[10] did
not fix the issue.

If the change didn't fix it, then maybe it's not related to that.


What's even worse is that adding any output to push(), even a puts("hi")
that does not print the argument or any of the stack vars, fixes it. So
something magic is going on inside the GCC optimization, I'm afraid this
is above my pay grade.
Could you send the file miscutils/dc.o that is created with and without 
this puts("hi") in push()?

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

Re: dc hitting a compiler bug, or undefined behavior

[Ralf Friedl]

Lauri Kasanen wrote:

On Sun, Mar 30, 2014, at 18:26, Ralf Friedl wrote:

What's even worse is that adding any output to push(), even a puts("hi")
that does not print the argument or any of the stack vars, fixes it. So
something magic is going on inside the GCC optimization, I'm afraid this
is above my pay grade.

Could you send the file miscutils/dc.o that is created with and without
this puts("hi") in push()?

Attached.
Are you using some special compiler options, especially regarding 
parameter passing in registers and stack alignment?


The relevant part of fail-dc.o is this:
 :
   0:   dd 07   fldl   (%edi)
The function expects the value to push at the address pointed to by 
%edi. But the functions that call push pass the value at the top of the 
CPU stack (not to be confused with the stack dc implements).


The relevant part ofsuccess-dc.o is this:
 :
   0:   57  push   %edi
   1:   8d 7c 24 08 lea0x8(%esp),%edi
   5:   83 e4 f0and$0xfff0,%esp
   8:   ff 77 fcpushl  -0x4(%edi)
   b:   55  push   %ebp
   c:   89 e5   mov%esp,%ebp
   e:   57  push   %edi
   f:   83 ec 14sub$0x14,%esp
  12:   dd 07   fldl   (%edi)
These lines set up an aligned stack.
0: save %edi
1: put address of top of stack at the time the function was called in 
%edi. This is the address of the parameter.

5: align stack to 0x10 boundary
8: push return address of the function
b, c: normal frame setup
e: save %edi for later use
f: make space for a double and align to 0x10
12: load parameter, %edi still points to the address of the parameter.

The instruction at 12 loads the double from address %edi after %edi has 
been set to point to the parameter area. The instruction at 0 in the 
failed case is exactly the same, except that %edi has not been setup 
before. So I would consider this a compiler bug.


I wrote that the instruction at f makes space for an aligned double. 
This is itself is strange because later on the double that is loaded 
from %edi is saved on the CPU stack and later loaded from the CPU stack 
and saved in the dc stack, which is unnecessary. Also the double is 
always loaded to the FPU stack and then removed if bb_error_msg_and_die 
is called, instead of loading it only after it is clear that it will be 
used. So there is also opportunity for further optimization of the compiler.


This stack alignment makes your code bigger, and the additional 
instructions also have to be executed, which also takes time. I'm not 
sure whether the aligned stack saves enough time to offset this.

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

Re: dc hitting a compiler bug, or undefined behavior

[Ralf Friedl]

Lauri Kasanen wrote:

On Mon, Mar 31, 2014, at 0:37, Ralf Friedl wrote:

Are you using some special compiler options, especially regarding
parameter passing in registers and stack alignment?

None, I'm afraid. CFLAGS etc are all unset. See later on for the gcc
defaults too.

Thanks for the research. I only speak x86 asm on a "yep, that's asm"
level.


The instruction at 12 loads the double from address %edi after %edi has
been set to point to the parameter area. The instruction at 0 in the
failed case is exactly the same, except that %edi has not been setup
before. So I would consider this a compiler bug.

Would it be possible for you to submit a gcc bug? I can try to do so
too, but it'll take a (long) while before I have time to build latest
gcc to test.
And if they ask what to do to reproduce the bug then I answer that I 
don't know. Besides I don't know whether 4.7.1 is still in development. 
They might ask whether an update to 4.7.3 or 4.8.2 solves the problem.

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

Re: Receiving two REMOVE actions for the same USB drive

[Ralf Friedl]

Mason wrote:

Mason wrote:

Hello everyone,

I'm using a vendor-supplied software stack which includes a
"customized" Linux kernel (2.6.28) along with busybox v1.20.2

I'm seeing something strange from mdev when I unplug my USB
mass storage device: the REMOVE action is notified twice
with two different DEVPATH.

# cat /etc/mdev.conf
$SUBSYSTEM=block 0:0 660 */application/usb_event.sh
$SUBSYSTEM=net   0:0 660 */application/usb_event.sh

# cat /application/usb_event.sh
printenv >> /tmp/stdout
echo "+++" >> /tmp/stdout

# cat superfloppy.insert
ACTION=add
NPARTS=0
HOME=/
SEQNUM=302
MAJOR=8
MDEV=sda
DEVPATH=/devices/platform/OEM-ehci-3.2/usb3/3-1/3-1:1.0/host1/target1:0:0/1:0:0:0/block/sda
SUBSYSTEM=block
PATH=/sbin:/bin:/usr/sbin:/usr/bin
MINOR=0
PWD=/dev
DEVTYPE=disk
+++

# cat superfloppy.remove
ACTION=remove
NPARTS=0
HOME=/
SEQNUM=310
MAJOR=8
MDEV=sda
DEVPATH=/devices/platform/OEM-ehci-3.2/usb3/3-1/3-1:1.0/host1/target1:0:0/1:0:0:0/block/sda
SUBSYSTEM=block
PATH=/sbin:/bin:/usr/sbin:/usr/bin
MINOR=0
PWD=/dev
DEVTYPE=disk
+++
ACTION=remove
NPARTS=0
HOME=/
SEQNUM=311
MAJOR=8
MDEV=sda
DEVPATH=/sda
SUBSYSTEM=block
PATH=/sbin:/bin:/usr/sbin:/usr/bin
MINOR=0
PWD=/dev
DEVTYPE=disk
+++

As you can see, the only difference between the two "remove" actions
is the DEVPATH (and the SEQNUM of course).

DEVPATH=/devices/platform/OEM-ehci-3.2/usb3/3-1/3-1:1.0/host1/target1:0:0/1:0:0:0/block/sda
DEVPATH=/sda

This double notification is annoying because I must filter it.
I suppose this is not coming from mdev, but from the kernel?

Any idea why the kernel would notify twice?

Any idea on how to fix this problem?

Are there any tools I can use to better diagnose the problem?

Could someone with a similar kernel test insert/delete of a USB
mass-storage device, and tell me if you are notified twice for
the removal?

I think this weird behavior may have been introduced by the OEM's
patches, but I have no easy way of testing.

Am I right to suspect mdev has nothing to do with the problem?

If mdev notifies twice, then it must mean that the kernel generates
two notifications, right?

Is anyone familiar with this part of kernel source, who could point
me where to look for the notification code?

You are rigtht, this has nothing to do with busybox.
If you say that simple filtering of the events would be annoying, you 
don't want to change the kernel to avoid the double notification.


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

Re: [PATCH 1/1] hwclock: Verify RTC file descriptor; use reentrant functions

[Ralf Friedl]

Bryan Evenson wrote:

Similarly, the returned pointer of ctime and localtime was
being used without checking if the pointer was valid. One of
these calls was causing a call to hwclock to enter an
uninterruptable sleep when the invalid hwclock access occurred.


An uninterruptable sleep is a kernel bug. Can you use strace on your 
script to see the sequence of calls that leads to this uninterruptable 
sleep?

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

Re: [PATCH] Add Linux gpio sysfs applets

[Ralf Friedl]

Sascha Hauer wrote:

This adds applets for manipulating gpios under Linux via sysfs. It uses
the /sys/class/gpio API to set direction and value of gpios and to read
back the actual value. The applets work like the corresponding C functions
in the kernel:

gpio_set_value  
gpio_get_value 
gpio_direction_output  
gpio_direction_input 

 is the Linux gpio number and  is 0 for low and 1 for high.
gpio_get_value will report the value to stdout.

If not already exported the applets export the gpio via
/sys/class/gpio/export. After usage the gpio is unexported again,
but only if it wasn't exported before calling the applet.
What is the advantage of these applets compared to a direct echo to 
/sys/class/gpio/... ?

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

Re: [PATCH 0/2] fix find_execable function and which code cleanup

[Ralf Friedl]

Tito wrote:

while trying to cleanup the debianutils/which command I've spotted
a minor glitch in bb's find_execable function as it is not able to handle
zero lenght prefixes in the PATH variable:

http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
8.3 Other Environment Variables
PATH
  snip
A zero-length prefix is a legacy feature that indicates the current
working directory. It appears as two adjacent colons ( "::" ), as an
initial colon preceding the rest of the list, or as a trailing colon
following the rest of the list. A strictly conforming application shall
use an actual pathname (such as .) to represent the current working
directory in PATH .

The real which command (really a script) in debianutils 4.4 does in fact
handle  zero-length prefixes in PATH correctly:

   for ELEMENT in $PATH; do
 if [ -z "$ELEMENT" ]; then
  ELEMENT=.
 fi

So PATCH 1 actually fixes this, the size impact is huge
and therefore it is dependent on CONFIG_DEKTOP:
Why is the size impact so huge? Why don't you use the equivalent of the 
script, if ELEMENT empty the ELEMENT="."?

I think the line

if (*p == ':' && p[1]  != '.')
is even wrong and should be
if (*p == ':')

What about this change:

while (p) {
   n = strchr(p, ':');
if (n)
*n++ = '\0';
+if (*p == '\0')
+p = ".";
-if (*p != '\0') { /* it's not a PATH="foo::bar" 
situation */

...
-}
The size impact should be the assignment 'p = "."' and the constant ".", 
about 10 bytes total, no extra processing, no extra malloc.

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

Re: [PATCH 0/2] fix find_execable function and which code cleanup

[Ralf Friedl]

Tito wrote:

On Thursday 01 May 2014 21:30:25 Ralf Friedl wrote:

Tito wrote:

while trying to cleanup the debianutils/which command I've spotted
a minor glitch in bb's find_execable function as it is not able to handle
zero lenght prefixes in the PATH variable:

http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
8.3 Other Environment Variables
PATH
   snip
A zero-length prefix is a legacy feature that indicates the current
working directory. It appears as two adjacent colons ( "::" ), as an
initial colon preceding the rest of the list, or as a trailing colon
following the rest of the list. A strictly conforming application shall
use an actual pathname (such as .) to represent the current working
directory in PATH .

The real which command (really a script) in debianutils 4.4 does in fact
handle  zero-length prefixes in PATH correctly:

for ELEMENT in $PATH; do
  if [ -z "$ELEMENT" ]; then
   ELEMENT=.
  fi

So PATCH 1 actually fixes this, the size impact is huge
and therefore it is dependent on CONFIG_DEKTOP:

Why is the size impact so huge? Why don't you use the equivalent of the
script, if ELEMENT empty the ELEMENT="."?

Hi,
because it is tricky to perform the same thing that IFS=: does.


I think the line

  if (*p == ':' && p[1]  != '.')
is even wrong and should be
  if (*p == ':')

No, if you remove && p[1]  != '.' it will process
also a PATH=:: that were transformed to PATH=:.:
in the previous pass.

Actually it should do that. "PATH=::" is equivalent to "PATH=.:.:."
If you don't remove it, it won't process PATH=:./subdir
There is a similar problem for PATH=dirwithdot.:


What about this change:

  while (p) {
 n = strchr(p, ':');
  if (n)
  *n++ = '\0';
+if (*p == '\0')
+p = ".";
-if (*p != '\0') { /* it's not a PATH="foo::bar"
situation */
  ...
-}
The size impact should be the assignment 'p = "."' and the constant ".",
about 10 bytes total, no extra processing, no extra malloc.


I also thought it was that easy in the beginning, but if I recall it
correctly in this way it was not working. With your changes:

export PATH=:/usr/local/bin:/usr/bin:/bin::/usr/local/games:/usr/games:
debian:~/Desktop/SourceCode/busybox$ ./busybox which -a  busybox
/bin/busybox

but it should show also 3 times the busybox binary in the cwd.

I don't know exactly what you did, but for me it works:
$ git diff
diff --git a/libbb/execable.c b/libbb/execable.c
index 178a00a..41540dc 100644
--- a/libbb/execable.c
+++ b/libbb/execable.c
@@ -37,6 +37,8 @@ char* FAST_FUNC find_execable(const char *filename, 
char **PATHp)

n = strchr(p, ':');
if (n)
*n++ = '\0';
+   if (*p == '\0')
+   p = ".";
if (*p != '\0') { /* it's not a PATH="foo::bar" 
situation */

p = concat_path_file(p, filename);
if (execable_file(p)) {
$ PATH=:/bin::/usr/bin: ./busybox which -a busybox
./busybox
./busybox
./busybox

To avoid a warning about "." being const, it should be written as
--- a/libbb/execable.c
+++ b/libbb/execable.c
@@ -37,14 +37,12 @@ char* FAST_FUNC find_execable(const char *filename, 
char **PATHp)

n = strchr(p, ':');
if (n)
*n++ = '\0';
-   if (*p != '\0') { /* it's not a PATH="foo::bar" situation */
-   p = concat_path_file(p, filename);
+   p = concat_path_file(*p ? p : ".", filename);
if (execable_file(p)) {
*PATHp = n;
return p;
}
free(p);
-   }
p = n;
} /* on loop exit p == NULL */
return p;


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

Re: [PATCH 0/2] fix find_execable function and which code cleanup

[Ralf Friedl]

Tito wrote:


That's it about PATCH 1.
There is still PATCH 2
Your patch searches in PATH for dir/file if dir/file is not executable. 
The normal behavior is to not search PATH if the argument contains a '/'.


If you rewrite "which" anyway, you could also change the following points:
- Remove default PATH, saves some bytes
- Output message if not found (maybe if DESKTOP)
- Return number of commands not found as per man page (Which returns the 
number of failed arguments, or -1 when no `programname´ was given)

$ which -v
GNU which v2.20, Copyright (C) 1999 - 2008 Carlo Wood.
$ (unset PATH; /usr/bin/which -a sh bash; echo $?)
/usr/bin/which: no sh in ((null))
/usr/bin/which: no bash in ((null))
2

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

Re: [PATCH 1/1] hwclock: Verify RTC file descriptor; use reentrant functions

[Ralf Friedl]

Bryan Evenson wrote:
Well, this is leading into some interesting questions about 
responsibility. I reported my results on the linux-arm-kernel mailing 
list, and it is possible I uncovered a bug in the Atmel RTC driver. 
I'll know a little more by this time tomorrow on that issue. I'd say 
Busybox should have reasonable assumptions about the kernel interfaces 
acting as documented. If that's the case, I'm not sure there is a need 
for this patch. Any thoughts? Either way, I'll report if I find out if 
a RTC driver bug was the root cause of my problems.
It is most likely a problem related to the platform because others 
couldn't reproduce in on PC hardware.
Also it is by definition a kernel bug if a process enters a state where 
if can't be killed.

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

Re: Changing the user name

[Ralf Friedl]

Laszlo Papp wrote:

is this possible? I am looking for something like "usermod -l" on desktop.

Alternatively, I have to look into the get/setpwent syscalls?

You can also use sed to change /etc/passwd

sed -i -e /s^olduser:/newuser:/ /etc/passwd
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Changing the user name

[Ralf Friedl]

Laszlo Papp wrote:

On Wed, May 14, 2014 at 7:28 AM, Ralf Friedl  wrote:

Laszlo Papp wrote:

is this possible? I am looking for something like "usermod -l" on desktop.

Alternatively, I have to look into the get/setpwent syscalls?

You can also use sed to change /etc/passwd

sed -i -e /s^olduser:/newuser:/ /etc/passwd

Yeah, plus I need to move (i.e. rename) the home folder and then I
think I am done. Thanks.

You should note that usermod doesn't do that anyway:
   -l, --login NEW_LOGIN
   The name of the user will be changed from LOGIN to 
NEW_LOGIN. Nothing else is changed. In particular, the user's home 
directory or mail spool should probably be renamed manually to reflect 
the new login name.


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

Re: Proposition: use a hashtable instead of bsearch to locate applets

[Ralf Friedl]

Rich Felker wrote:

On Fri, May 30, 2014 at 12:13:41PM +0100, Laurent Bercot wrote:

On 05/30/2014 10:16 AM, Bartosz Gołaszewski wrote:

I've checked the times just by looking up all the applets in a loop
and measuring the time using gettimeofday() - the results are: ~220
microseconds for bsearch and ~150 microseconds for hashtable on my
linux laptop. Is it significant? I think so. Is it noticeable?
Probably not. The idea came to me, when thinking about unifying the
hashtables used in busybox. I guess you're right and it isn't really
worth the size increase.

  I think it would definitely be a worthwhile improvement if all busybox
is doing was looking up the applets in a loop. ;)
  A more realistic test, however, would be to fork+exec a trivial applet
(true, for instance) in a loop, and measure the times with bsearch vs.
with the hash table. And I'm certain you'll find that the difference
becomes quite insignificant.

If there is a desire to change the way applet lookup is done, I would
suggest trying to make it optimal in both size and performance. 150µs
is not impressive at all. A perfect hash constructed at build time for
the list of busybox applet-name strings should make it possible to do
the applet lookup in ~1µs with trivial code/table size (all constant
in .text).
The original number was 220µs for "looking up *all* the applets in a 
loop and measuring the time". So this is the time for an unspecified 
number of applets (maybe allyesconfig) on an unspecified machine. So if 
we assume that at least 50 applets were defined, the time for one applet 
is 4µs or less.


I just built busybox with 373 applets, the time to call the lookup 
100 times for APPLET_NAME(0) is 100ms on an AMD 4GHz CPU. An own 
binary search implementation saves a few ms, probably because the 
comparison function is inline which saves a call. It also adds a few 
bytes. I choose APPLET_NAME(0) because it is the worst case for the 
binary search. The best case is APPLET_NAME(NUM_APPLETS/2), which only 
needs 18ms for 100 calls, or 13ms with own binary search.


As busybox has a focus on size and the difference is small I think it 
doesn't make sense to change anything here.

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

Re: Issues removing files with certain characters in their names.

[Ralf Friedl]

Jason Cipriani wrote:

I just want to bring to attention a potential issue, described here:

http://superuser.com/questions/587778/cant-delete-file-with-foreign-letters

It seems at least two users (the original poster and also this post 
) were unable to remove a file 
that contained certain foreign characters in the filename, both using 
BusyBox.


In one case a user resorted to setting up a samba server then deleting 
the file remotely from Windows. In the other case the discovered 
workaround was to "rm -i *" and answer no to all files except the 
problematic one.


Not sure if this is BusyBox-specific issue, user error, or something 
else, and I don't have BusyBox to test, but I thought it was 
significant that both users with that problem were using it and am 
reporting it here in case anybody is interested in investigating.


This issue has nothing to do with busybox, it is related to the kernel 
and to non Linux file systems. A Linux file system should be able to use 
anything as the filename that doesn't exceed the length for a file entry 
and doesn't contain '/' or '\0'. It should provide back the exactly the 
same string with readdir. The kernel also make no assumption about an 
encoding of the file names.


When accessing other file systems, especially FAT and NTFS, this is not 
true, but it is not a problem busybox can solve. The kernel should be 
able to access and delete the files it has created on such a file system 
with the same settings, but there is no guarantee that the files on such 
a file system have been created with the same settings, of even by a 
Linux kernel. In the case of NTFS, it is now common to use the userspace 
NTFS file system, so it is not really a kernel issue.


Files are stored as UCS-2 on NTFS and VFAT but not on regular FAT file 
systems. In this case NTFS is mentioned, but no information on mount 
options and in general not enough information to reproduce the problem.

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

Re: setting a default LANG option for unicode, ash shell - patch

[Ralf Friedl]

Richard Moore wrote:

I am just posting this in case it helps someone else out.

I had trouble creating a small live boot system to run an ncurses 
program requiring unicode support, running straight from rcS.
Without the LANG env set it didn't display any unicode. Exporting LANG 
in rcS didnt have an effect.
I had partial success starting it via a new shell from rcS and 
exporting it the variable but ultimately if the program was exited and 
then restarted, the env was gone.


This little patch sets a default utf8 LANG option for the ash shell so 
you don't need to try and export it, it will just work from the get 
go. Hope it is of some use to someone.
This patch may be useful to you, and you could probably get the same 
effect by writing it into some configuration file, but in general it is 
just a bad idea.


Busybox or ash should not assume a specific value for LANG, and 
certainly it should not set a value for LANG. It should not overwrite a 
value that is already present in the environment, and it also should not 
set a value if LANG was not set. In short, it should not modify LANG at all.

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

Re: [PATCH 1/2] find: use sysconf(_SC_ARG_MAX) to determine the command-line size limit

[Ralf Friedl]

Bartosz Gołaszewski wrote:

2014-06-22 13:49 GMT+02:00 Denys Vlasenko :

+ IF_FEATURE_FIND_EXEC_PLUS(G.max_argv_len = BB_ARG_MAX;) \

The "- 2048" part should be here, not inside BB_ARG_MAX
(not all users will want that subtraction).

But then the user would substract these bytes even when using ARG_MAX
or 32*1024 - when it's not needed as ARG_MAX already includes space
for the environment.
According to a comment from findutils/xargs.c the 2048 comes from the 
standard that for some reason limits the size of argv+environ to 
ARG_MAX-2048 bytes.


ARG_MAX is the total space available for all arguments. All arguments 
include the environment, so the space needed for the environment is not 
available for argv. Of course the space needed for the environment may 
be smaller or much larger than 2048, so the 2048 is just a hack. The 
correct approach would be to determine the size of the actual 
environment and use that.
The size for the arguments is not defined in the standard: " It is 
implementation-defined whether null terminators, pointers, and/or any 
alignment bytes are included in this total."
Two likely implementations are to pass only the strings and reconstruct 
the pointers to the strings after that, or to pass the strings and the 
pointers (argv and environ). That means each string needs either 
strlen()+1 bytes or strlen()+1+sizeof(char*) bytes, so the latter would 
be safe to assume.
At the moment, busybox xargs completely ignores the environment, so the 
reason that xargs seems to work is that either 2048 is enough for the 
environment (not likely) or that 32*1024 is an upper bound and not a 
lower bound for the argument size:

  n_max_chars = 32 * 1024;
  arg_max = sysconf(_SC_ARG_MAX) - 2048;
  if (arg_max > 0 && n_max_chars > arg_max)
n_max_chars = arg_max;

Are there any systems in use today where ARG_MAX is smaller than 32kB?


+
+ /* TODO Introduce a growable buffer and use BB_ARG_MAX macro to
+  * determine a safe value for n_max_chars. Remove this check afterwards.
+  */

I don't understand this comment.

Before commit f92f1d0 there was a comment in findutils/xargs.c:

/* -s NUM default. fileutils-4.4.2 uses 128k, but I heasitate
  * to use such a big value - first need to change code to use
  * growable buffer instead of fixed one.

The small value of 32*1024 is used here because the command-line
buffer is allocated in a single kzalloc() call at line 559. This is
why we do ' n_max_chars = 32 * 1024;'  at line 536 if the value
returned by bb_arg_max() is greater than 32*1024, and - for now -
'n_max_chars -= 2048;' at line 541 is not needed.

If we were to use bb_arg_max() we'd need to use a growable vector just
like 'find -exec {} +' does right now.
The buffer is allocated with xzalloc and for some reason with size 
(n_max_chars + 1).
Is there a reason to clear the buffer? It is not cleared before reuse 
anyway. So on a MMU machine it would be possible to use a much bigger 
default with xalloc, and the pages would only be mapped as needed. This 
also wouldn't need any code to grow the buffer, and would also save the 
time needed to copy the contents of the buffer on realloc.


As the functionality from "xargs" and "find -exec {} +" is quite 
similar, it might be a good idea to define some functions that can be 
used by both commands.

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

Re: ftpd authentication [PATCH] ftpd: NOMMU/chroot fix

[Ralf Friedl]

Denys Vlasenko wrote:

On Thu, Jun 26, 2014 at 11:45 AM, Morten Kvistgaard
 wrote:

I've attached my strace.

I'm not sure that it tells me anything though.
I've run: sudo strace -p -f -o ftpd.strace
Is there a better way?

Anyway, the current code will exit jail with the following code:

...
if (fchdir(G.root_fd) != 0)
 _exit(127);
...

But on my Ubuntu and uClinux that's not enough to break out of jail.

What do you mean? In your strace, fchdir succeeds:

15144 fchdir(3 
15143 <... mmap2 resumed> ) = 0xb77d
15144 <... fchdir resumed> )= 0


And so the following code will fail:

...
/* + 1: we must use relative path here if in chroot.
* For example, execv("/proc/self/exe") will fail, since
  * it looks for "/proc/self/exe" _relative to chroot!_ */
execv(bb_busybox_exec_path + 1, (char**) argv);
_exit(127);
...


The strace might reflect this:
...
execve("proc/self/exe", ["ftpd", "-l", "/"], [/* 9 vars */]) = -1 ENOENT (No 
such file or directory)
...

This is strange. Any ideas why this fails on your machine?

Morten Kvistgaard wrote:

But on my Ubuntu and uClinux that's not enough to break out of jail. And so the 
following code will fail:
I have no idea why soneone would use NOMMU on Ubuntu, but most likely 
busybox is dynamically linked and therefor exec busybox fails because it 
doesn't find the dynamic linker. Even if it would find the dynamic 
linker, the linker wouldn't find the dynamic libraries.


Why does the child process only chdir and no chroot?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Info for Rich Felker, as I have permanent failure to deliver mail to him

[Ralf Friedl]

Harald Becker wrote:

Hi Rich,

you told me dal...@libc.org had an intermediate problem, but shall 
work normal. Since my first question to this, I have continued trouble 
to send mail to your address. I always get an error reply, like this:


Harald
This is an error in the DNS of libc.org. As you can see, the MX record 
points to a CNAME and not to an A record. See also RFC 2181.

libc.org.   1800IN  MX  10 mail.aerifal.cx.
mail.aerifal.cx.86400   IN  CNAME brightrain.aerifal.cx.

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

Re: [PATCH v2] top: fix and merge code to parse /proc/meminfo

[Ralf Friedl]

Timo Teras wrote:

On Sun, 27 Jul 2014 21:50:28 +0200
Denys Vlasenko  wrote:

Applied, thanks!

Thanks, though I noticed now a weirdness that did not happen before.

+static void parse_meminfo(unsigned long meminfo[MI_MAX])
+{
...
+   memset(meminfo, 0, sizeof(meminfo));

Seems to not work. The sizeof() return something small. Probably
sizeof(unsigned long*).

Is my compiler broken, or is the expected behaviour?
That is the expected behaviour. A parameter declaration of unsigned 
long[] is that same as unsigned long*.

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

Re: [Bug: Busybox 1.22.1] false return 0 instead of 1 with '--help' switch.

[Ralf Friedl]

Xabier Oneca -- xOneca wrote:

Hello again,

2014-09-05 18:19 GMT+02:00 Xabier Oneca  --  xOneca :

Hello list,

I want to highlight what GNU coreutils' help docs (info coreutils
'false invocation') says about false when invoked with --help:


Note that `false' (unlike all other programs documented herein)

exits unsuccessfully, even when invoked with `--help' or `--version'.

I think Busybox should do the same with the applet.

And I want to add that --help will only be used ever in an interactive
shell by a person, so the return value may be dispensable. (i.e.
doesn't mind to the user how does --help return as soon as the help
text is printed in screen)
Actually this is the argument to leave it as it is, because nobody cares 
about the return value of "false --help".


It might be nice if "false --help" would return false, but it would add 
extra code to busybox for no real benefit.

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

Re: [Bug: Busybox 1.22.1] false return 0 instead of 1 with '--help' switch.

[Ralf Friedl]

Xabier Oneca -- xOneca wrote:

>> And I want to add that --help will only be used ever in an interactive

>> shell by a person, so the return value may be dispensable. (i.e.
>> doesn't mind to the user how does --help return as soon as the help
>> text is printed in screen)
>
> Actually this is the argument to leave it as it is, because nobody 
cares about the return value of "false --help".


What I was trying to say is that if a script appends --help to 
/bin/false because of a poorly sanitized user input, it should still 
return 1. And this change in behaviour wont not break any script nor 
user interaction with the applet. (Maybe even fixes some scripts!)


Do you have an example of any real word script that passes any arguments 
(user supplied or not) to /bin/false ?

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

Re: Reg: Busybox mount fails to mount ntfs file system

[Ralf Friedl]

Indira Valmiki wrote:

I am using debian wheezy.
Kernel version - 3.4.74.
busybox version - 1.21.1

I am trying to mount a NTFS filesystem USB by using following command.

mount -t ntfs-3g /dev/sda1 

But i'm getting below error,

mount: mounting /dev/sda1 on /var/local/devices/ntfs failed: No such 
device

Do you have mount.ntfs-3g in /sbin or elsewhere?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: rm -r fails to delete entire hierarchy when path goes in and out of it

[Ralf Friedl]

Gian Ntzik wrote:

It seems that using rm -r with a path that goes into the hierarchy
intended for removal (and back up e.g. using dot-dots) fails to remove
the entire hierarchy.

For example,

$ mkdir -p /tmp/a/b/c
$ mkdir -p /tmp/a/e
$ rm -r /tmp/a/b/../../a
rm: can't remove 'a/b/../../a/e': No such file or directory
rm: can't remove 'a/b/../../a': No such file or directory

GNU rm does this:
$ mkdir -p /tmp/a/b/c /tmp/a/e
$ rm -r /tmp/a/b/../../a
rm: cannot remove '/tmp/a/b/../../a': No such file or directory

In general I think it is not a good idea to pass such arguments to rm.

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

Re: In regard to CVE-2014-7169 CVE-2014-6271

[Ralf Friedl]

Sean Mathews wrote:
In regard to CVE-2014-7169 CVE-2014-6271 looking at 
busybox-1.22.1/networking/udhcp/dhcpc.c line 403 fill_envp() it seems 
as if it would be trivial to mess with bootfile and inject a packet 
that has garbage in the bootfile and exploit this vulnerability.
We should keep in mind that this is a vulnerability in bash, not in 
busybox. It is unlikely that a system with busybox uses bash as the 
default shell, and it is unlikely that a system would update busybox, 
but not bash, so it wouldn't make much sense to add bash protection to 
busybox. I also assume that busybox bash compatibility doesn't extend to 
importing shell functions.


In general, I wonder why someone would think importing shell functions 
is a good idea, and if there is anyone who relies on this feature. I 
looked at the bash man page, and only found this:

> INVOCATION
> ...
>   If  the shell is started with the effective user (group) id not 
equal to the real user (group) id,  ..., shell functions are not 
inherited from the environment.
This would imply that they are imported otherwise, but it is likely that 
people would read the start of the sentence and skip the rest because 
the don't use setuid/setgid.


In http://seclists.org/oss-sec/2014/q3/771 (bug-bash-gnu) it has been 
argued that importing shell functions is a feature that almost nobody 
uses and that should be off by default. I agree with this.
Why should a non-interactiv shell import shell functions from the 
environment? When the shell executes a script, that script can define 
all the shell functions it needs.
Any why would an interactive shell import shell functions? The 
interactive shell already reads several startup files, these files can 
be used to define all the desired shell functions.

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

Re: [PATCH 2/3] zcip: allow our own class B range to be used for ZeroConf

[Ralf Friedl]

Michel Stam wrote:

169.254 may already be used by a local network. This patch allows
specifying your own IP range.
Why would you want to use a different range? You would have to specify 
it to each client, which seems against the idea of ZeroConf.

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

Re: [PATCH] Config: select PLATFORM_LINUX if using sendfile()

[Ralf Friedl]

Bartosz Golaszewski wrote:

Man entry for sendfile:

   Not specified in POSIX.1-2001, or other standards.

   Other UNIX systems implement sendfile() with different  semantics  and
   prototypes. It should not be used in portable programs.

Select PLATFORM_LINUX if enabling FEATURE_USE_SENDFILE.

Signed-off-by: Bartosz Golaszewski 
---
  Config.in | 1 +
  1 file changed, 1 insertion(+)

diff --git a/Config.in b/Config.in
index 285fe0a..07b4bf3 100644
--- a/Config.in
+++ b/Config.in
@@ -267,6 +267,7 @@ config PAM
  config FEATURE_USE_SENDFILE
bool "Use sendfile system call"
default y
+   select PLATFORM_LINUX
help
  When enabled, busybox will use the kernel sendfile() function
  instead of read/write loops to copy data between file descriptors

I suggest ddoing it the other way, enable sendfile only for Linux.
As it is, sendfile is enabled by default, which would automatically 
select Linux, which might select other things that don't work on 
non-Linux systems.




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

Re: why are init's arguments wiped ?

[Ralf Friedl]

Nicolas CARRIER schrieb:

"/proc/1/cmdline is not a standard Unix ..."

I absolutely don't know any other OS, be it Unix or not... But don't 
they provide a way to access the command-line used to launch a program ?
In busybox's code at least, I see only one implementation of 
read_cmdline, which reads the content of /proc/[PID]/cmdline.
From that I conclude that only Unices having this file can have a 
busybox ps implementation dealing with the command-line.
Busybox is primarily intended for Linux. While some of the applets are 
Posix compliant, others are very Linux specific.


Traditionally, Unix doesn't provide a way to access the comand-line of 
another program. Programs like ps used to do it anyway, but they had to 
be set-uid to root so that they could access the memory of another 
process. The whole /proc filesystem is not part of traditional Unix.


What if I want to debug how init was launched ? The way it behaves now 
makes it impossible to know if a runlevel has been passed, how it was 
passed, or even if we are launching the right init binary.
Assuming you use Linux, which you do, you can use /proc/cmdline to see 
the Kernel command line. From this you can determine which init was use. 
Either it was specified, or the default was used. It also contains the 
runlevel that was passed to init.


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

Re: busybox sed, 'r' command

[Ralf Friedl]

Cristian Ionescu-Idbohrn wrote:

sed (GNU sed) 4.2.2 can do this:

$ printf 'foo
bar
baz' | sed r -
foo
bar
baz

or, after storing the text in a file:

$ printf 'foo
bar
baz' >/tmp/bar

$ sed r /tmp/bar
foo
bar
baz

But busybox sed can't:

$ printf 'foo
bar
baz' | busybox sed r -
sed: empty filename

$ busybox sed r /tmp/bar
sed: empty filename

$ printf '' | busybox sed 'r /tmp/bar'


$ busybox sed 'r /tmp/bar'


The 'r' command is documented by GNU sed as a GNU extension.  Still,
busybox sed documents the 'r' command as supported:

r [address]r file
   Read contents of file and append after the contents of the
   pattern space. Exactly one space must be put between r and the
   filename.

Am I misinterpreting the documentation?

From the documentation:
>   The full format for invoking `sed' is:
> sed OPTIONS... [SCRIPT] [INPUTFILE...]
So in your example you invoce sed with the script "r" and the input file 
"-" or "/tmp/bar". The content is not printed because it is the argument 
to the "r" command, but because it is the main input file to sed. You 
can avoid that by using quotes around the command and the file name, or 
by omitting the space between the command and the filename.
You should also try the last two examples, where you invoke busybox sed 
with quotes, with GNU sed. The behaviour is the same.


You should note that in your example when reading from a file, sed 
didn't read from stdin, at least you don't mention it, although your 
interpretation would mean that the filename is the argument to the "r" 
command, therefor no argument is given to sed, and sed should read stdin.


You should also not that invoking the "r" command with the filename 
causes the content of this file to be inserted after every line. When 
reading from a pipe, the pipe is empty after the first line.


My documentation to GNU sed 4.2.2 says:
> `r FILENAME'
>  As a GNU extension, this command accepts two addresses.
>
>  Queue the contents of FILENAME to be read and inserted into the
>  output stream at the end of the current cycle, or when the next
>  input line is read.  Note that if FILENAME cannot be read, it is
>  treated as if it were an empty file, without any error indication.
>
>  As a GNU `sed' extension, the special value `/dev/stdin' is
>  supported for the file name, which reads the contents of the
>  standard input.

So the main difference seems to be that GNU sed doesn't give an error 
message if the file can't be read. I'm not sure why that would be a good 
idea.
Also not that there is no mention of using "r -" for stdin, instead 
/dev/stdin is mentioned.


On the other hand, I don't know why busybox sed needs exactly one space 
between command and filename. GNU sed works with zero or more spaces.

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

Re: Parallel make bug in busybox

[Ralf Friedl]

Arnout Vandecappelle wrote:

As an aside, there may be a second issue: applets/Kbuild.src specifies the same
command for both include/applet_tables.h and include/NUM_APPLETS.h, but that
command will always generate both files. So if the rules for the two files are
launched in parallel, there is another problem. The easiest way to work around
that is to (arbitrarily) let include/NUM_APPLETS.h depend on
include/applet_tables.h with an empty command:

include/NUM_APPLETS.h: include/applet_tables.h ;

It's not entirely accurate, but in practice it works.
There is a perfectly valid syntax to express that one command generates 
more than one file:

diff --git a/applets/Kbuild.src b/applets/Kbuild.src
index b612399..5d4c53e 100644
--- a/applets/Kbuild.src
+++ b/applets/Kbuild.src
@@ -40,8 +40,5 @@ include/usage_compressed.h: applets/usage 
$(srctree_slash)applets/usage_compress

 quiet_cmd_gen_applet_tables = GEN include/applet_tables.h
   cmd_gen_applet_tables = applets/applet_tables 
include/applet_tables.h include/NUM_APPLETS.h


-include/applet_tables.h: applets/applet_tables
-   $(call cmd,gen_applet_tables)
-
-include/NUM_APPLETS.h: applets/applet_tables
+include/applet_tables.h include/NUM_APPLETS.h: applets/applet_tables
$(call cmd,gen_applet_tables)

This should also fix the problem with the parallel build. The original 
form would invoke the same command twice, maybe at the same time. The 
second form will invoke it only once.


The problem with include/NUM_APPLETS.h is that it is not rebuild by 
applets/Kbuild.src if include/NUM_APPLETS.h is removed, but 
include/applet_tables.h is not. This can be fixed by adding 
include/NUM_APPLETS.h to the dependencies of applets/usage_pod, although 
it is not really a dependency.

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

Making DO_POSIX_CP configurable

[Ralf Friedl]

Hi Denys

Can you point to real security problems from the use of cp with POSIX 
semantics?

I know, the target of the copy operation could be a symbolic link to 
some other file that would be overwritten. This would require the 
attacker to have write permissions to the target directory and would 
require cp to be used without the -i option. Normally, only /tmp is 
world writable, and there is not much reason to copy files from 
elsewhere to /tmp.
As most systems come with a POSIX compatible cp program, I think it 
would be widely known if that was a serious security risk.

If you really think it is a security risk to write to the user specified 
file, that would also be the case for every other program that writes to 
a file.

Especially, by first unlinking the file, you break the following 
assumptions in contrast to POSIX Semantic:
- If the target file is a special file (block, character or pipe), the 
special file is replaced with a regular file.
- The owner and permissions of the target file are not preserved.
- properties like acl oder user_xattr of the target file are not preserved.
- hard links of the target file are not preserved.

Regards
Ralf Friedl

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Making DO_POSIX_CP configurable

[Ralf Friedl]

> User comes to you and says "I accidentally deleted my most important
> directory. I know that you make daily backups. Can you restore
> it from backup?"
>
> You do
>
> cp -a /backup/home/user/dir /home/user
>
> But user has crafted it so that backup contains
> dir/many_more_dirs/innocuous_file, and he also
> created a symlink
>
> ln -s /etc/passwd /home/user/dir/many_more_dirs/innocuous_file
>
> Now imagine the effect of the above cp command.
>   
Personally, I would never restore a backup over an existing directory, 
but to an empty one. From there I (or the user) could move the needed 
files to the right place.

But I see your point.
> The attacker don't write file himself. He tricks root into doing it.
>   
The attacker creates the link and then must tick root into writing to 
it. That was clear.
> GNU coreutils have cp --remove-destination. I think people
> will forget to use it until it's too late.
>
> I see that for "cp file1 file2" it is a problem,
> but for "cp -r dir1 dir2" it is exactly what you want. right?
>   
So "cp -r" would imply "--remove-destination", while "cp without -r" 
would not?

I think that is a good solution, better that differentiating between 
regular and device files.

Regards
Ralf Friedl

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

busybox 1.6.1 miscutils/taskset problem with glibc, wrong number of params

[Ralf Friedl]

> Glibc's  sched_setaffinity only has two args
> extern int sched_setaffinity (__pid_t __pid, __const cpu_set_t *__mask)
>
> While uclibc has 3.
>   

My glibc-2.3.4 and glibc-2.4 both have three args (pid, cpusetsize, mask)


___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

nsupdate

[Ralf Friedl]

Hi

I'm looking for something like nsupdate (Update DNS records, RFC2136).

Is there something like that in busybox? If not, should it be included?

I compiled nsupdate from bind for my target system, but the binary is 
larger that 1MB.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: nsupdate, brctl

[Ralf Friedl]

Mike Frysinger wrote:
>> If not, should it be included? 
>> 
> anything that "should" be in busybox already is ... anything beyond that 
> (like 
> what you want) is merely left up to people to contribute ... if it doesnt 
> suck and is generally useful, it'll get added ... if it bitrots and/or sucks, 
> it'll get dropped
>   
What I wanted to say is this:
I want something like nsupdate, and I want it to be much smaller that 1MB.
Now I can write something that does just the updates I want to do, or I 
could add some input parsing and turn it into a generic program like 
nsupdate.

The question is, would you include a nsupdate applet or not? If you 
think it doesn't belong to busybox, it would make no sense for me to 
write a generic program.

A related question, do you think it would be useful to have a brctl 
utility in busybox to configure Linux network bridges? The standalone 
brctl is about 40k, so I don't know whether there is much space to be 
saved by including it, but it might be convenient to have it if the 
network setup includes bridges.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Patch for less, page forward

[Ralf Friedl]

Hi

Could you add the key 'f' for page forward in less?
I'm used to the keys 'f' and 'b' for page forward and backward.
The difference in size should be minimal if different at all.
Most likely the compiler will generate a jump table anyway, so there 
would be no difference in the genereted size.

Regards
Ralf Friedl


--- miscutils/less.c
+++ miscutils/less.c
@@ -1135,7 +1135,7 @@
case KEY_UP: case 'y': case 'k':
buffer_up(1);
break;
-   case PAGE_DOWN: case ' ': case 'z':
+   case PAGE_DOWN: case ' ': case 'z': case 'f':
buffer_down(max_displayed_line + 1);
break;
case PAGE_UP: case 'w': case 'b':

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: pgrep/pkill

[Ralf Friedl]

Tito wrote:
> Maybe this version could be more useful.
>
> void xfree(void *ptr)
> {
>   /*   sets the pointer to NULL
>   after it has been freed.*/
>   void **pp = (void **)ptr;
>
>   if (*pp == NULL) return;
>
>   free(*pp);
>   *pp = NULL;
> }
>
>   
You only modify the argument of xfree, not the original pointer.

What you wanted to write is:

void xfree(void **pp)
{
/*   sets the pointer to NULL after it has been freed.*/
if (*pp == NULL) return;
    free(*pp);
*pp = NULL;
}


Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Problem with httpd and POST data for cgi, and patch

[Ralf Friedl]

Hi

I found a problem with httpd and POST data for cgi programs.

get_line() is used to read the header. This function uses hdr_buf with 
hdr_ptr and hdr_cnt to keep track of the current position. For cgi 
programs cgi_io_loop_and_exit() is called to transfer the POST data to 
the cgi and the result back.

The request variable Content-Length contains the length of the POST 
data. cgi_io_loop_and_exit() tries to send (hdr_cnt + post_len) bytes to 
the cgi. But at this point, the actual header is already read, so 
whatever is left in hdr_cnt ist actally not the header, but (part of) 
the POST data.

In the following (simplified) example, assuming the whole request was 
read in one block, after reading the header, hdr_cnt is 10, and post_len 
is also 10. httpd will write the 10 bytes from hdr_ptr to the cgi (which 
is correct) and then wait for additional 10 bytes (post_len) from the 
network, which will never come, as they are already received. The cgi 
program won't receive EOF and, depending on the program, wait forever 
for more input.

So the bytes transferred from hdr_cnt should be subtracted from 
post_len. I'm not sure whether it might be a legitimate reason for 
(hdr_cnt > post_len),  but I included a check for this just in case.

The patch is against SVN 20228.

Regards
Ralf Friedl


POST /cgi-bin/test
Content-Length: 10

1234567890



--- networking/httpd.c~ 2007-10-12 14:38:42.0 +0200
+++ networking/httpd.c  2007-10-12 14:50:44.0 +0200
@@ -1102,6 +1102,10 @@
if (count > 0) {
hdr_ptr += count;
hdr_cnt -= count;
+   if (post_len >= count)
+   post_len -= count;
+   else
+   post_len = 0; /* should not 
happen, maybe check for post_len < hdr_cnt? */
} else {
/* EOF/broken pipe to CGI, stop piping 
POST data */
hdr_cnt = post_len = 0;

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Problem with httpd and POST data for cgi, and patch

[Ralf Friedl]

Hi

I just found out that my previous patch only works for POST data smaller 
that hdr_buf. This patch should also work for larger POST data.
> I found a problem with httpd and POST data for cgi programs.
>
> get_line() is used to read the header. This function uses hdr_buf with 
> hdr_ptr and hdr_cnt to keep track of the current position. For cgi 
> programs cgi_io_loop_and_exit() is called to transfer the POST data to 
> the cgi and the result back.
>
> The request variable Content-Length contains the length of the POST 
> data. cgi_io_loop_and_exit() tries to send (hdr_cnt + post_len) bytes to 
> the cgi. But at this point, the actual header is already read, so 
> whatever is left in hdr_cnt ist actally not the header, but (part of) 
> the POST data.
>
> In the following (simplified) example, assuming the whole request was 
> read in one block, after reading the header, hdr_cnt is 10, and post_len 
> is also 10. httpd will write the 10 bytes from hdr_ptr to the cgi (which 
> is correct) and then wait for additional 10 bytes (post_len) from the 
> network, which will never come, as they are already received. The cgi 
> program won't receive EOF and, depending on the program, wait forever 
> for more input.
>
> So the bytes transferred from hdr_cnt should be subtracted from 
> post_len. I'm not sure whether it might be a legitimate reason for 
> (hdr_cnt > post_len),  but I included a check for this just in case.
>
> The patch is against SVN 20228.
>
> Regards
> Ralf Friedl
>
>
> POST /cgi-bin/test
> Content-Length: 10
>
> 1234567890
>
>   
--- networking/httpd.c
+++ networking/httpd.c
@@ -1051,6 +1051,10 @@
 * and send it to the peer. So please no SIGPIPEs! */
signal(SIGPIPE, SIG_IGN);

+   if (post_len >= hdr_cnt)
+   post_len -= hdr_cnt;
+   else
+   post_len = 0;
/* NB: breaking out of this loop jumps to log_and_exit() */
out_cnt = 0;
while (1) {

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Problem with httpd and POST data for cgi, and patch

[Ralf Friedl]

Denys Vlasenko wrote:
> I propose just doing
>
> post_len -= hdr_cnt;
>
> instead of that if(). If post_len will get negative, this code
> will catch that and will close CGI's input fd:
>
> if (toCgi_wr) {
> pfd[TO_CGI].fd = toCgi_wr;
> if (hdr_cnt > 0) {
> pfd[TO_CGI].events = POLLOUT;
> } else if (post_len > 0) {
> pfd[0].events = POLLIN;
> } else {
> /* post_len <= 0 && hdr_cnt <= 0:
>  * no more POST data to CGI,
>  * let CGI see EOF on CGI's stdin */
> ==> close(toCgi_wr);
> toCgi_wr = 0;
> }
> }
>
> What do you think?
As there is no difference between (post_len == 0) and (post_len < 0), I 
think you are right.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Ash + telnetd: telnet client hangs after exit

[Ralf Friedl]

Alexander Kriegisch wrote:
> BB 1.7.2 and many old versions down to 1.4.x, I don't know exactly.
> Platform: mipsel
>
> We have had this problem for months, maybe 1-2 years, with different BB
> versions, but now I think it is time to report it:
>
> On my embedded system there is BB running telnetd. The shell is ash. A
> client logs in and works until he types "exit". After that the session
> is terminated, but the telnet client hanngs waiting for - I don't know
> what. I can reproduce it with several telnet clients. It does not happen
> with Dropbear sshd, so I strongly assume it is a BB problem.
>
> Interestingly, the session exits cleanly if I call "setconsole -r"
> during the telnet session.
>   
> Maybe somebody can find out what is wrong here. It might be a similar
> case to "ash endless loop after ssh client is killed" (April 20-29,
> 2007), I don't know.
>
> Regards
There are two possible conditions for a login server (telnetd, 
SSH-Server, rlogin server, ...) to use as the end of the session:
1. The immediate child process (the shell) exits.
2. All child processes have closed the client side of the pty.
(The client closing the network connection is of course also causes the 
end of the session, but that is not relevant here.)

busybox telnetd and the OpenSSH Server use variant 2, the dropbear 
server uses variant 1.

There are reasons for both variants, so it is a matter of taste which 
should be used.

With variant 2, the connection stays open as long as any child process 
has the client pty open. It is possible to run "tail -f logfile & exit" 
and leave the connection open until the program exits (in this case 
never) or the client closes the connection. This may be annoying in case 
it wasn't intended to leave a program running.

With variant 1, the connection is closed immediately after the shell exits.

With the OpenSSH Server, the connection may also be used to forward 
other connections over the encrypted channel, this is not the case with 
busybox telnetd.
I also tried the normal telnetd server on my Linux system, it also uses 
variant 1, so probably busybox telnetd should do the same.

To do that, it would be necessary to catch SIGCHLD, which seems to be 
left to default now. Then with the pid of the child, find the correct 
session and shut it down.

The normal telnet server also just closes the ptmx instead of a SIGKILL 
to the child process. The closing of the ptmx causes a SIGHUP, which 
gives the shell the opportunity to a clean up and exit.

I think I could make a patch for this if it is determined that busybox 
telnetd should be changed to do that.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Ash + telnetd: telnet client hangs after exit

[Ralf Friedl]

Denys Vlasenko wrote:
> On Monday 15 October 2007 15:15, Alexander Kriegisch wrote:
>   
>> BTW, the output "ps" output diff looks like this on i386:
>>
>> $ diff -U0 ps1.txt ps2.txt
>> --- ps1.txt 2007-10-15 16:13:06.0 +0200
>> +++ ps2.txt 2007-10-15 16:13:25.0 +0200
>> @@ -130 +129,0 @@
>> -root 31704 31643  0 16:10 ?00:00:00 [login] 
>> 
>
> Aha. You have a login which does not exec shell, it spawns it
> as a child. When shell exits, login exits too.
>
> What looks strange to me is that you see a _zombie_ login.
> It means that it exited, but is not waited for yet.
>
> How come telnetd doesn't see EOF from login's fd? It *exited*,
> and that implicitly closes all fds! I'm puzzled.
>   
telnetd doesn't see EOF on the pty side because the client pty is open 
by a child of the shell, not the shell itself.

The reason why the process is a zombie is that telnetd will only wait 
for the child after sending SIGKILL, and it will only send SIGKILL after 
either the network connection or the client side of the pty is closed. 
But in this case the problem is that the client pty is not closed.
> (1) can you check PPID of zombie login? Is it 1 or ?
>   
Therefor the PPID should be telnetd. Also, if the PPID would be init 
that would mean that init is not working, which is unlikely.
> (2) is it possible that you start telnetd so that it inherits
> "ignore SIGCHLD" from the parent? Try adding this line
> after signal(SIGPIPE, SIG_IGN):
>
>  signal(SIGPIPE, SIG_IGN);
> +signal(SIGCHLD, SIG_DFL);
>   
SIGCHLD should already be SIG_DFL. Maybe you meant SIG_IGN, but ignoring 
SIGCHLD should change nothing except for the left of zombie process.
> Unrelated note: I looked at telnetd source and tightened up
> some loose ends. Can you test this patch? (I don't think
> it will help with this particular problem, though...)
>   
The only change in your patch which seems relevant to this problem is 
where you removed the SIGKILL before the wait(). I think this is 
dangerous in a single-threaded server. The child shell will probably 
eventually clean up and exit, but if that takes some seconds, all other 
connections will hang for that time.

As I wrote earlier, the problem is that the shell exits but the client 
pty remains open. You can reproduce this as follows:

telnet server
# login ...
# on the server:
sleep 10 & exit

The telnet client will only exit after sleep terminates.
SIGCHLD is ignored (as in SIG_DFL) at the moment and the session will 
only be terminated when the client pty or the network connection is closed.

Now the question is whether this should be considered a bug or a feature.

If it is considered a bug, the solution is to remove the sigkill and the 
wait, install a signal handler for SIGCHLD, inside the signal handler 
find the tsession for the pid and close the session.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Ash + telnetd: telnet client hangs after exit

[Ralf Friedl]

Denys Vlasenko wrote:
> No, I not only removed SIGKILL, I also removed wait:
>
> /*kill(ts->shell_pid, SIGKILL);
> wait4(ts->shell_pid, NULL, 0, NULL);*/
>
> I don't wait for anything now. I tear down connections as soon
> as there is a problem (or EOF) reading/writing to the pty.
>   
I didn't look careful enough.
May I suggest using #if 0 ? That makes it easier to read, and it also 
works with embedded comments.

#if 0
kill(ts->shell_pid, SIGKILL);/* Works even with comments */
wait4(ts->shell_pid, NULL, 0, NULL);
#endif


> Ralf, can you review attached patch?
>   
I updated to SVN 20262, which I assume is the same as the attached 
patch. It works with my tests.

As Busybox is about reduced code size, also some unrelated remarks:

In free_session():
/* error if ts->sockfd_read == ts->sockfd_write. So what? ;) */
-close(ts->sockfd_write);
...
-if (maxfd < ts->sockfd_read)
-maxfd = ts->sockfd_read;
if (maxfd < ts->sockfd_write)
maxfd = ts->sockfd_write;
Actually, ts->sockfd_read == ts->sockfd_write, unless telnetd is called 
from inetd, in which case free_session is not called at all, so that 
close is not necessary. The same for testing maxfd against both 
sockfd_read and sockfd_write.

In make_new_session():
-if (sock_r > maxfd) maxfd = sock_r;
ts->sockfd_read = sock_r;
ndelay_on(sock_r);
Similar to the previous, either ts->sockfd_read == ts->sockfd_write, or 
in the case called by inetd, ts->sockfd_read < ts->sockfd_write 
(ts->sockfd_read == 0, ts->sockfd_write == 1).
If you apply this, it would be useful to add comments explaining the 
reasons for this.

In telnetd_main():
/* Ignore trailing NUL if it is there */
if (!TS_BUF1[ts->rdidx1 + count - 1]) {
-if (!--count)
-goto skip3;
+--count;
}
ts->size1 += count;
ts->rdidx1 += count;
if (ts->rdidx1 >= BUFSIZE) /* actually == BUFSIZE */
ts->rdidx1 = 0;
}
 skip3:
If (count == 0), the execution of the following lines has no effect. It 
takes a little longer, but at least my telnet client doesn't send 
trailing NULs most of the time. The question is code size or execution time.

In free_session() and other places, there are loops like this:
/* Scan all sessions and find new maxfd */
ts = sessions;
while (ts) {
...
ts = ts->next;
}
This can be written as:
/* Scan all sessions and find new maxfd */
for (ts = sessions; ts; ts = ts->next) {
...
}
The generated code should be the same, but the loop control is all in 
one line. This is mainly a matter of preferences. I consider the one 
line version easier to read.

It is especially useful in cases like the loop in telnetd_main, where it 
wouldn't be necessary to write "ts = next;" before each "continue;" I 
don't know whether the compiler is clever enough to detect the common 
"ts = next; continue;", so in that case it may also save a few instructions.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Ash + telnetd: telnet client hangs after exit

[Ralf Friedl]

Denys Vlasenko wrote:
> ...
> function old new   delta
> telnetd_main13551350  -5
> make_new_session 532 521 -11
> free_session 118 101 -17
> --
> (add/remove: 0/0 grow/shrink: 0/3 up/down: 0/-33) Total: -33 bytes
>textdata bss dec hex filename
>  6763282538   12104  690970   a8b1a busybox_old
>  6762952538   12104  690937   a8af9 busybox_unstripped
>
> Please review attached (or see current svn).
It seems fine.

I'm surprised that your changes to telnetd_main removed only 5 bytes. 
The compiler seems to be quite clever already. But with your changes it 
is also obvious to the human reader that kill_session does the same each 
time.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

long / short options (was Re: start-stop-daemon does not recognise --start)

[Ralf Friedl]

Denys Vlasenko wrote:
> Try this patch.
> ...
Do you think it would be useful to define something like this:

#define OPTION_HELP_TEXT(SHORT, LONG, TEXT) SHORT 
USE_FEATURE_LONG_OPTIONS(LONG) TEXT

Then the options could be written as

OPTION_HELP_TEXT(" -v", ", --verbose", " Verbose\n")

and the middle Part could be included/excluded as necessary. That would 
avoid duplicating the description for the "long option" / "no long 
option" cases and make it easier to maintain.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: patch - ping select interface.

[Ralf Friedl]

Bob Keyes wrote:
> I originally made this for my own needs, contributed it to openwrt but 
> think it should go upstream to busybox. I am new here, not sure of the 
> regular way I am supposed to make contributions, but whoever is in charge 
> of accepting patches please take a look at 
> https://dev.openwrt.org/ticket/2522 which will explain the rational for 
> the feature and includes the patch for it.
>
> Regards,
> Bob Keyes
Can you explain how this is different from option -I ? From the man page 
of the linux ping:
-I interface address
Set source address to specified interface address. Argument may be 
numeric IP address  or  name  of  device.
When pinging IPv6 link-local address this option is required.

It seems the -I is supposed to accept either an IP address or a device 
name. I suggest to keep it compatible.

Regards
Ralf Friedl

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: patch - ping select interface.

[Ralf Friedl]

Bob Keyes wrote:
> On Fri, 19 Oct 2007, Ralf Friedl wrote:
>> Bob Keyes wrote:
>>> I originally made this for my own needs, contributed it to openwrt 
>>> but think it should go upstream to busybox. I am new here, not sure 
>>> of the regular way I am supposed to make contributions, but whoever 
>>> is in charge of accepting patches please take a look at 
>>> https://dev.openwrt.org/ticket/2522 which will explain the rational 
>>> for the feature and includes the patch for it.
>> Can you explain how this is different from option -I ? From the man 
>> page of the linux ping:
>> -I interface address
>> Set source address to specified interface address. Argument may be 
>> numeric IP address  or  name  of  device.
>> When pinging IPv6 link-local address this option is required.
>>
>> It seems the -I is supposed to accept either an IP address or a 
>> device name. I suggest to keep it compatible.
> in Linux (net-tools) ping this is what -I does. But not in busybox 
> ping. -I is used for another option in busybox ping.
Busybox ping says:
-I iface/IP Use interface or IP address as source

iputils ping says:
-I interface (or) address

So both seem to agree. The problem might be that in busybox ping the -I 
is not fully implemented. Busybox ping even contains code to detect 
whether the argument to -I is a device or an address. It just seems that 
if_index is only used for IPV6 ping and not for IPV4 ping.

In iputils ping the code for option -I is like this:
if (optarg is IP-Address)
...
else
setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, optarg, 
strlen(optarg)+1)

This seems to be exactly what your code does for option -n. I suggest to 
do the same with the option -I, as it was already intended and to keep 
both versions of ping compatible.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Patch for adduser: Create a system user

[Ralf Friedl]

Hi

I have a patch for adduser to support the option "-S: Create a system 
user", which is presently accepted but ignored.

Also I changed the range for normal users to start at 1000 instead of 
500, which is what most current distributions do.

As a side note:
It is not possible to create users with group 0. The code seems to 
create a new group with same and same number as the new user.
Why would that be useful?
Also, I think it is possible that this implementation results in 
duplicate user ids:
The first loop, "while (!fgetpwent_r(..." find a free user id.
The second loop, "while (getgrgid(p->pw_uid)) p->pw_uid++;" increments 
uid until it finds an unused group id.
It is possible that this incremented uid is in use as a user id.

Regards
Ralf Friedl

Index: loginutils/adduser.c
===
--- loginutils/adduser.c
+++ loginutils/adduser.c
@@ -11,6 +11,7 @@
 #include "libbb.h"

 #define OPT_DONT_SET_PASS  (1 << 4)
+#define OPT_SYSTEM_ACCOUNT (1 << 5)
 #define OPT_DONT_MAKE_HOME (1 << 6)


@@ -18,14 +19,24 @@
 /* EDR recoded such that the uid may be passed in *p */
 static int passwd_study(const char *filename, struct passwd *p)
 {
-   enum { min = 500, max = 65000 };
FILE *passwd;
/* We are using reentrant fgetpwent_r() in order to avoid
 * pulling in static buffers from libc (think static build here) */
char buffer[256];
struct passwd pw;
struct passwd *result;
+   int min, max;

+if (option_mask32 & OPT_SYSTEM_ACCOUNT) {
+   min = 1;
+   max = 999;
+   }
+   else {
+   min = 1000;
+   max = 65000;
+   }
+
+
passwd = xfopen(filename, "r");

/* EDR if uid is out of bounds, set to min */
Index: include/usage.h
===
--- include/usage.h
+++ include/usage.h
@@ -27,7 +27,7 @@
"   -g GECOSGECOS field\n" \
"   -s SHELLLogin shell\n" \
"   -G GROUPAdd user to existing group\n" \
-   "   -S  Create a system user (ignored)\n" \
+   "   -S  Create a system user\n" \
"   -D  Do not assign a password\n" \
"   -H  Do not create home directory"

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Patch for adduser: Create a system user

[Ralf Friedl]

Tito wrote:
> maybe this could fix the problem you report:
> ...
>   /* create new gid always = uid and re-check if the uid is free 
> */
>   while (getgrgid(p->pw_uid) && getpwuid(p->pw_uid))
>   p->pw_uid++;
>   
I think this shoud use "or", not "and":

while (getgrgid(p->pw_uid) || getpwuid(p->pw_uid))
p->pw_uid++;

Personally, I don't use this feature, it was only an observation that it 
might be possible to create duplicate ids if this feature is used.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Problem with static ARP on router TP-Link TD8840

[Ralf Friedl]

dytu iuyr wrote:
> My router TP-Link TD8840 uses Busybox v.1.00 and there is ARP tool on 
> it. I can  add new entry using: "arp add IP Adress MAC Adress". The 
> problem is that I can't save that entry permanently. After rebooting 
> that entry is missing...
> What am i doing wrong? Is it common problem of Busybox or maybe its only 
> problem with my version  of busybox and my arp applet? What can I do?
>   
The "problem" is that you expect these settings to be permanent. You 
(hopefully) don't expect IP-adresses or routing entries to be permanent 
but use init scripts to set them up at boot time. You should do the same 
if you need permanent ARP entries.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Ash + telnetd: telnet client hangs after exit

[Ralf Friedl]

Hi

I found some more problems with the current telnetd code (I'm sorry that 
I didn't notice them earlier).

The bigger problem is that without the option -K, telnetd will never 
wait for its children, leaving them as zombies.

A minor problem is that it is possible, although unlikely, that two 
children exit at the same time. Or more exactly, it is possible that one 
child exits, which raises SIGCHLD for telnetd. If another child exits 
after the first, but before the signal was delivered, the signal handler 
will be called only once, not twice for the two children.
If the system is heavy loaded, maybe parts of telnetd paged out, this 
time may be log enough for this to happen.
It is unlikely to happen with telnetd, but I already had this problem 
with a mail server in real life. The solution is to loop in the signal 
handler until wait finds no more children.

I also changed the argument to waitpid from &sig to NULL because "man 
waitpid" says: "If status is not NULL, wait or waitpid store ...", 
because the status is not used.

Regards
Ralf Friedl


--- networking/telnetd.c
+++ networking/telnetd.c
@@ -384,13 +384,17 @@
pid_t pid;
struct tsession *ts;

-   pid = waitpid(-1, &sig, WNOHANG);
-   if (pid > 0) {
+   for (;;) {
+   pid = waitpid(-1, NULL, WNOHANG);
+   if (pid <= 0)
+   break;
+   if (!(option_mask32 & OPT_WATCHCHILD))
+   continue;
ts = sessions;
while (ts) {
if (ts->shell_pid == pid) {
ts->shell_pid = -1;
-   return;
+   break;
}
ts = ts->next;
}
@@ -464,8 +468,7 @@
/* We don't want to die if just one session is broken */
signal(SIGPIPE, SIG_IGN);

-   if (opt & OPT_WATCHCHILD)
-   signal(SIGCHLD, handle_sigchld);
+   signal(SIGCHLD, handle_sigchld);

 /*
This is how the buffers are used. The arrows indicate the movement

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Ash + telnetd: telnet client hangs after exit

[Ralf Friedl]

Denys Vlasenko wrote:
> I noticed another bug: we do not restore SIGCHLD/SIGPIPE to default.
>
> As to "zombies created without -K" problem, I guess
> signal(SIGCHLD, SIG_IGN);
> will prevent that, no need to have handler installed/called.
>   
Ignoring SIGCHLD should work. According to the man page, on execve() 
signals set to be caught are reset to default, while SIGCHLD set to 
SIG_IGN is undefined. The behavior of other signals set to SIG_IGN is 
not even mentioned, so restoring them to default is the safe solution.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: /etc/passwd and symlinks

[Ralf Friedl]

Paul Fox wrote:
> denys wrote:
>  > 
>  > Thus I propose introducing xmalloc_readlink_recursive()
>  > which does not suffer from those two problems,
>  > and using it here (and elsewhere: insmod, syslogd -
>  > grep for xmalloc_readlink).
>
> well, i've written a new version, but it doesn't do what you
> thought it was going to.  :-)
>
> realpath() fully canonicalizes a path -- it makes it absolute,
> removes "/..", and expands symlinks all along the path.
>
> readlink() (and also xmalloc_readlink()) simply returns the value
> of the named symlink.  (which makes the switch from realpath()
> to xmalloc_readlink() somewhat suspicious, by the way.)
>   
I think that should be no problem. The path will eventually point to a 
real file, and as long as the new temporary file is created in the same 
directory, it should be possible to rename it later.

Also, I don't see why insmod should need the realpath of the object. I 
thought the basename of the object file is used as the module name, but 
I have also seen few cases where the module name was not the file name, 
so that seems to be only a convention and not mandatory.
> i've written xmalloc_readlink_follow(char *path) which iteratively
> does textual expansion of path, expanding symlinks at the tail end
> of path.  no other canonicalization is done, and the result is not
> necessarily absolute.  this is the minimum needed for my "allow
> /etc/passwd to be a symlink" change, and it's probably useful for
> most of the other current uses of xmalloc_readlink().
>   
I noticed that you test against MAXSYMLINKS only in the case of relative 
symlinks. I consider this inconsistent.
In the case of /etc/passwd, the administrator probably can be trusted to 
not create a loop. But suppose you have an absolute symlink /etc/passwd 
-> /etc/passwd. The function would loop forever, while a stat on 
/etc/passwd would fail.with ELOOP. With a relative symlink /etc/passwd 
-> passwd the loop would be detected.

Another question is what the function should do if the target of the 
link does not exist. xmalloc_readlink will always return the target 
name, whether it exists or not. I don't know whether this behavior would 
be useful for xmalloc_readlink_follow or not.

Regards
Ralf Friedl
> /*
>  * this routine is not the same as realpath(), which canonicalizes
>  * the given path completely.  this routine only follows trailing
>  * symlinks until a real file is reached, and returns its name. 
>  * intermediate symlinks are not expanded.  as above, a malloced
>  * char* is returned, which must be freed.
>  */
> char *xmalloc_readlink_follow(const char *path)
> {
>   char *buf = NULL, *lpc, *linkpath;
>   int bufsize;
>   smallint looping = 0;
>
>   buf = strdup(path);
>   bufsize = strlen(path) + 1;
>
>   while(1) {
>   linkpath = xmalloc_readlink(buf);
>   if (!linkpath) {
>   if (errno == EINVAL) /* not a symlink */
>   return buf;
>   free(buf);
>   return NULL;
>   } 
>
>   if (*linkpath == '/') {
>   free(buf);
>   buf = linkpath;
>   bufsize = strlen(linkpath) + 1;
>   } else {
>   bufsize += strlen(linkpath);
>   if (looping++ > MAXSYMLINKS) {
>   free(linkpath);
>   free(buf);
>   return NULL;
>   }
>   buf = xrealloc(buf, bufsize);
>   lpc = bb_get_last_path_component_strip(buf);
>   strcpy(lpc, linkpath);
>   free(linkpath);
>   }
>   }
> }
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: /etc/passwd and symlinks

[Ralf Friedl]

Paul Fox wrote:
> ralf wrote:
>  > 
>  > Also, I don't see why insmod should need the realpath of the object. I 
>  > thought the basename of the object file is used as the module name, but 
>  > I have also seen few cases where the module name was not the file name, 
>  > so that seems to be only a convention and not mandatory.
>
> so you're saying the current code in insmod.c, and even a change to
> use the _follow() version of readlink, would be okay, right?  just making
> sure i understand you.  :-)
>   
No, I didn't at all look at the insmod code. Actually this seems to 
refer to the code for 2.4 kernels, and I haven't used 2.4 kernels for 
some time.
As realpath was used in add_ksymoops_symbols, I suppose ksymoops might 
also work with the unmodified filename, as the kernel would follow the 
same symlinks and eventually arrive at the same file.
But I don't know and I don't have a kernel 2.4 or the utilities for the 
older kernels to try it.

>  > I noticed that you test against MAXSYMLINKS only in the case of relative 
>  > symlinks. I consider this inconsistent.
>
> oops, you're right.  that's an oversight, which i'll fix.  (note
> that the test isn't really guaranteed -- there may be more
> undetected symlinks embedded in the middle of the path, since i'm
> only checking the tail.  but it will be caught eventually.)
>   
symlinks in the path may cause ELOOP in readlink. That should be no 
problem, the final path would case ELOOP anyway.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: tr segfaults

[Ralf Friedl]

Fernando Silveira wrote:
> Yep, but the patch follows inline. The read(2) return value was being
> stored in an unsigned variable, avoiding the error detection. I also
> added an error message, which wasn't implemented.
>   
Maybe tr should not exit in this case with EXIT_SUCCESS, but return an 
error? Especially as tr is most of the time used in shell scripts, where 
the return code is more likely to be noticed than an error message.

GNU tr returns 1:
$ tr a b < /tmp; echo $?
tr: read error: Is a directory
1

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: tar segfaults (busybox 1.8.1)

[Ralf Friedl]

Denys Vlasenko wrote:
> It means that you built your glibc with some weird debugging mode,
> and it converts
>
> strcpy(array, string);
>
> into some sort of
>
> strcpy_with_overrun_check(array, sizeof(array), string);
>
> In this case, it blew up despite code was correct.
>
> It also makes all your strcpy's bigger.
>
> Which version of glibc does this? With which configure options
> did you built it?
>   
glibc can't to this, it must be the compiler, maybe with some special 
options.

This would also be consistent with the strace log, which shows that 
libgcc_s is loaded to display the stack trace. If glibc would do it, the 
functions for the backtrace would probably be included in glibc.

Regards
Ralf Friedl

29355 munmap(0xb7f29000, 4096)  = 0
29355 open("/dev/tty", O_RDWR|O_NOCTTY|O_NONBLOCK) = 7
29355 writev(7, [{"*** buffer overflow detected ***"..., 34}, {"./busybox", 9}, 
{" terminated\n", 12}], 3) = 55
29355 open("/etc/ld.so.cache", O_RDONLY) = 8
29355 fstat64(8, {st_mode=S_IFREG|0644, st_size=125457, ...}) = 0
29355 mmap2(NULL, 125457, PROT_READ, MAP_PRIVATE, 8, 0) = 0xb7f0b000
29355 close(8)  = 0
29355 access("/etc/ld.so.nohwcap", F_OK) = -1 ENOENT (No such file or directory)
29355 open("/lib/libgcc_s.so.1", O_RDONLY) = 8


___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: tar segfaults (busybox 1.8.1)

[Ralf Friedl]

Denys Vlasenko wrote:
> On Friday 16 November 2007 12:53, Ralf Friedl wrote:
>   
>> glibc can't to this, it must be the compiler, maybe with some special
>> options.
>> 
> With magic macro definition of strcpy, it can.
>   
You are right.
I found this in /usr/include/bits/string3.h:
#define strcpy(dest, src) \
  ((__bos (dest) != (size_t) -1)\
   ? __builtin___strcpy_chk (dest, src, __bos (dest))   \
   : __strcpy_ichk (dest, src))
static __inline__ char *
__attribute__ ((__always_inline__))
__strcpy_ichk (char *__restrict __dest, const char *__restrict __src)
{
  return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
}

So it seems to be some macro magic together with some compiler builtins. 
__bos seems to be __builtin_object_size
I also found this in /usr/include/string.h:
# if __USE_FORTIFY_LEVEL > 0 && !defined __cplusplus
/* Functions with security checks.  */
#  include 
# endif
#endif


So it seems that __USE_FORTIFY_LEVEL or _FORTIFY_SOURCE was defined when 
busybox was compiled.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Wget PASV parsing

[Ralf Friedl]

Richard Kojedzinszky wrote:
> I've been using busybox for a while, and ran into the problem when 
> using wget behind an openbsd gw, with nat and ftp-proxy. Without this 
> patch wget connects to the wrong host, so the operation fails. This 
> fixes that, but increases size by ~60 bytes in my environment. But 
> maybe it is worth to apply for full functionality. 
Could you describe what is wrong?
For all you write, it may as well be an error in the gateway that you 
try to fix in busybox.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Changes in udhcp

[Ralf Friedl]

Hi

I have two remarks to the recent changes to udhcp.

networking/udhcp/script.c contains "static int mton()" to count the bits 
of a network mask. This could use the ffs() function:
   The ffs() function returns the position of the first (least 
significant) bit set in the word i.  The least significant bit is 
position 1 and the most significant position is 32.

So mton could be written as
  return 32 - (ffs (ntohl(mask)) - 1);

Newer version of gcc also contain __builtin_clz which counts leading 
(high) zero bits. As we want to count leading one bits, we must first 
invert mask.
  return __builtin_clz (~ntohl(mask));

Both examples are not tested, so some modifications might be necessary.
__builtin_clz and probably also ffs use special CPU instructions where 
available, so they would be both faster and smaller in that case. 
Otherwise they probably call into libgcc, which  is present anyway.


In networking/udhcp/leases.c I found the comment "= { 0 } helps gcc to 
put it in rodata, not bss".
Why is it desirable to have it in rodata instead of bss? rodata consumes 
space in the file while bss doesn't.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: mount with Labels

[Ralf Friedl]

Jason Curl wrote:
> Is it possible to use BusyBox mount to mount a filesystem on boot depending 
> on the label of the device/partition? I have a Compact Flash disk that mounts 
> it's second partition but on some devices this is /dev/hda1 and on other 
> devices this i /dev/hdc1 (the root is /dev/hda2 or /dev/hdc2 so that when a 
> user takes the CF card out and puts it on Windows, they don't see the boot 
> partition, only the FAT partition securing the software from accidental 
> deletion).
>
> Using a label removes this dependency. Also I intend to allow users to insert 
> a USB key to my embedded device but I can't say what device it would be up 
> front, but I can easily specify the label of that partition.
I'm not sure about mounting at boot time, but at least for the USB key 
the solution might be to use hotplug.

I think that the devices present at boot time also generate "hotplug" 
events, but I never tried that.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: [PATCH] zombie processes (maybe was: init does not kill reparented processes)

[Ralf Friedl]

Harald Küthe wrote:
> Our inittab calls a startscript which is not ended during runtime, but
> only when the settopbox shuts down.
>   
I would consider this an application error and not a problem of init.

> There exists a patch which is hanging around in our development tree for
> ages now which we used to avoid zombie processes.
> This is maybe related to some old stuff found here:
> http://www.busybox.net/lists/busybox/2004-August/012422.html
>   
This patch will solve your zombie problem, but it will only work because 
you don't use any other of init's functions.

init will loop in waitfor(), waiting for your start script to stop, 
which it will not. The only difference is that with the patch init will 
wait for other processes. But init will not start any other processes 
that should run from inittab. This is probably not important for you 
because you don't have other actions in inittab.

But the solution to your problem is not to patch init but to run you 
"start" script in the background or to use "once" instead of "sysinit" 
in your inittab. "once" actions are started by init, but init doesn't 
wait for them.

Personally, I prefer to see the zombie processes as a strong indication 
for a problem with the start script to an init that doesn't run 
processes without any hint.

Regards
Ralf Friedl


___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: New version of tac.c

[Ralf Friedl]

Jean-Christophe Dubois wrote:
> just a few comments:
>
> On Tuesday 08 January 2008 22:42:23 Tito wrote:
>   
>> llist_add_to(&list, memcpy(size, &i, 
>> sizeof(int))); 
>> 
>> llist_add_to(&list, line); 
>> 
> More generally would it make sense to define a structure holding both the 
> string size and the string pointer? This would allow to call llist_add_to() 
> only once per line instead of twice (same for llist_pop) ...
>   
I agree that it would be more efficient to use a structure for the 
length and and the content of the line, for memory usage and for run time.
And because llist_add_to() allocates a new llist_t element for each list 
element, it would use even less memory and less calls to malloc() to use 
a struct that contains the list pointer, the line length and the line 
content. It might even result in a little smaller code size. A malloc() 
is necessary anyways, and the code to inser an element in a single 
linked list may be smaller that the code to call a function to do it.

Also, "size" has been allocated with malloc(), so it should be properly 
aligned to be accessed as an int.
Therefor
  memcpy(size, &i, sizeof(int))
could be written as
  *size = i

which is better readable and most likely produces shorter code, 
especially on machines where unaligned writes are not possible.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Again, german umlauts in ash command line prompt

[Ralf Friedl]

Alexander Griesser wrote:
> I'm still stuggling with getting the german umlauts (ä,ö,ü, a.s.o)
> working on my thinclient installation.
> The really strange thing is, that it doesn't seem to be a keyboard
> layout problem at all.
>
> When I'm at the ash command prompt, I can _NOT_ type ö, ä, ü and what
> is really strange, I cannot type the § character.
>
> In the following output I tried to enter these characters in the first
> line (you can't see them, because they didn't get accepted).
> In the second line, I used the `read` builtin to fill a variable with
> input and that worked perfectly.
>
> I can reproduce this every time and on all my systems (even with a
> freshly compiled busybox on my desktop system), so I'm sure it has
> nothing todo with keyboard layout settings, language settings, etc.
>
> ~/busybox/busybox-1.9.0 $ ll"!$"$"$
> ~/busybox/busybox-1.9.0 $ read TEST
> äöü"!§$
> ~/busybox/busybox-1.9.0 $
>
> Can anyone reproduce this too?
>   
Yes. I also have strace output:

read(0, "\303", 1)  = 1
read(0, "\244", 1)  = 1
read(0, "\303", 1)  = 1
read(0, "\266", 1)  = 1
read(0, "\303", 1)  = 1
read(0, "\274", 1)  = 1
read(0, "\344", 1)  = 1
read(0, "\366", 1)  = 1
read(0, "\374", 1)  = 1
read(0, "t", 1) = 1
write(1, "t", 1t)= 1
read(0, "e", 1) = 1
write(1, "e", 1e)= 1
read(0, "s", 1) = 1
write(1, "s", 1s)= 1
read(0, "t", 1) = 1
write(1, "t", 1t)= 1

It seems that the characters are simply ignored, at least there is no 
echo for these characters.
I tried both UTF-8 and ISO encoding.

The "read" builtin doesn't use the line editor, therefor all characters 
are displayed correctly and also assigned to the input variable.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: typing ^V to a shell

[Ralf Friedl]

I just noticed with current SVN that Ctrl-U doesn't work, because it 
depends on CONFIG_FEATURE_EDITING_FANCY_KEYS.
As Ctrl-U is a feature that works even without line editing, I consider 
that strange. Especially because Ctrl-W, which is also present without 
line editing, is always enabled.

As a side note, input of characters with codes above 128 works now, and 
input of control characters with Ctrl-V also works, but control 
characters are output directly and not in readable form.

With Readline:
echo ^[[2J
With BB:
echo  { Screen erased while typing }

But considering the scope of BB it is reasonable to leave it as it is.

Regards
Ralf Friedl

Paul Fox wrote:
> i noticed a while ago that it was impossible to type a TAB
> character on the ash commandline.  by comparison, gnu readline
> allows prefixing any character with ^V to force insertion of the
> character -- this is now i normally enter a TAB when using bash. 
> there was code in lineedit.c to allow this, but it was ifdefed
> out along with unrelated features.  i've re-enabled it
> (unconditionally).  along with the fix for 8-bit characters, i
> think it's now possible to enter any character (with the possible
> exception of NULL) at the commandline.
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: stty: invalid number 'erase'

[Ralf Friedl]

Alexander Kriegisch wrote:
>> # stty erase 
>> stty: invalid number 'erase' 
>> # stty eof 
>> stty: invalid number 'eof'
>> 

I think the problem comes from copying find_mode to find_control in stty.c.

static const struct mode_info *find_mode(const char *name)
{
int i = 0;
const char *m = mode_name;

while (*m) {
if (strcmp(name, m) == 0)
return &mode_info[i];
m += strlen(m) + 1;
i++;
}
return NULL;
}

static const struct control_info *find_control(const char *name)
{
int i = 0;
-const char *m = mode_name;
+const char *m = control_name;

while (*m) {
if (strcmp(name, m) == 0)
return &control_info[i];
m += strlen(m) + 1;
i++;
}
return NULL;
}


Also there already is a function to return the index of a string 
(index_in_strings), so why not use it?

static const struct mode_info *find_mode(const char *name)
{
int i = index_in_strings(mode_name, name);
return i >= 0 ? &mode_info[i] : NULL;
}

static const struct control_info *find_control(const char *name)
{
int i = index_in_strings(control_name, name);
    return i >= 0 ? &control_info[i] : NULL;
}


Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: stty: invalid number 'erase'

[Ralf Friedl]

Bernhard Fischer wrote:
> On Tue, Jan 15, 2008 at 03:44:32PM +0100, Ralf Friedl wrote:
>   
>> Alexander Kriegisch wrote:
>> 
>>>> # stty erase 
>>>> stty: invalid number 'erase' 
>>>> # stty eof 
>>>> stty: invalid number 'eof'
>>>> 
>>>> 
>> I think the problem comes from copying find_mode to find_control in stty.c.
>> Also there already is a function to return the index of a string 
>> (index_in_strings), so why not use it?
>> 
>
> Can you please send a complete, tested patch?
Here it is.

Regards
Ralf Friedl

--- coreutils/stty.c~   2007-11-24 12:02:30.0 +0100
+++ coreutils/stty.c2008-01-15 15:40:32.0 +0100
@@ -780,30 +780,14 @@

 static const struct mode_info *find_mode(const char *name)
 {
-   int i = 0;
-   const char *m = mode_name;
-
-   while (*m) {
-   if (strcmp(name, m) == 0)
-   return &mode_info[i];
-   m += strlen(m) + 1;
-   i++;
-   }
-   return NULL;
+   int i = index_in_strings(mode_name, name);
+   return i >= 0 ? &mode_info[i] : NULL;
 }

 static const struct control_info *find_control(const char *name)
 {
-   int i = 0;
-   const char *m = mode_name;
-
-   while (*m) {
-   if (strcmp(name, m) == 0)
-   return &control_info[i];
-   m += strlen(m) + 1;
-   i++;
-   }
-   return NULL;
+   int i = index_in_strings(control_name, name);
+   return i >= 0 ? &control_info[i] : NULL;
 }

 enum {

___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: stty: invalid number 'erase'

[Ralf Friedl]

Bernhard Fischer wrote:
> On Tue, Jan 15, 2008 at 03:44:32PM +0100, Ralf Friedl wrote:
>   
>> Alexander Kriegisch wrote:
>> 
>>>> # stty erase 
>>>> stty: invalid number 'erase' 
>>>> # stty eof 
>>>> stty: invalid number 'eof'
>>>> 
>>>> 
>> I think the problem comes from copying find_mode to find_control in stty.c.
>> Also there already is a function to return the index of a string 
>> (index_in_strings), so why not use it?
>> 
>
> Can you please send a complete, tested patch?

I already sent this a week ago. Is something wrong with the patch or did it 
simply get lost?

Regards
Ralf Friedl

--- coreutils/stty.c~   2007-11-24 12:02:30.0 +0100
+++ coreutils/stty.c2008-01-15 15:40:32.0 +0100
@@ -780,30 +780,14 @@

static const struct mode_info *find_mode(const char *name)
{
-   int i = 0;
-   const char *m = mode_name;
-
-   while (*m) {
-   if (strcmp(name, m) == 0)
-   return &mode_info[i];
-   m += strlen(m) + 1;
-   i++;
-   }
-   return NULL;
+   int i = index_in_strings(mode_name, name);
+   return i >= 0 ? &mode_info[i] : NULL;
}

static const struct control_info *find_control(const char *name)
{
-   int i = 0;
-   const char *m = mode_name;
-
-   while (*m) {
-   if (strcmp(name, m) == 0)
-   return &control_info[i];
-   m += strlen(m) + 1;
-   i++;
-   }
-   return NULL;
+   int i = index_in_strings(control_name, name);
+   return i >= 0 ? &control_info[i] : NULL;
}

enum {


___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: reboot / umount ordering

[Ralf Friedl]

Kittlitz, Edward (Ned) wrote:
>
> Hello,
>
> I have trouble getting my ext3 disk filesystems unmounted during reboot.
>
> I don't know the names of all the processes, so I don't think I can 
> use "killall".  And another inittab "shutdown" action "/bin/umount -a 
> -r" can fail on certain filesystems; I don't know why yet.
>
> It seems to me that busybox shutdown_system() should be trying to 
> unmount everything after all processes have been killed.
>
You can try
kill -15 -1
sleep 2
kill -9 -1
umount -a -r

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Busybox make, modify my /dev/null on host

[Ralf Friedl]

Paul Fox wrote:
> why _shouldn't_ gcc remove the output file in
> that case?  if gcc removes the target of -o in all other cases,
> then, in my opinion, /dev/null shouldn't be special.  if it's
> important that gcc be able to do "test runs" without creating
> output, then there should be a "test run" option that says,
> "don't create an output file".  or maybe "-o -" should be
> implemented, to allow writing to stdout, so it can be redirected
> to /dev/null. 
>   
I agree that it should not be necessary that gcc checks for special 
files. Of course it would be even better if ld would not create the 
output file until it is complete, but it is really a very special case. 
Normally the output file is just a regular file, and therefor it is OK 
to remove it.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: ash r21030 broken?

[Ralf Friedl]

Cristian Ionescu-Idbohrn wrote:
> On Wed, 20 Feb 2008, Denys Vlasenko wrote:
>   
>> And if it still fails, make both alloc() functions zero out
>> allocated block:
>>
>> static void *
>> ckmalloc(size_t nbytes)
>> {
>> -   return ckrealloc(NULL, nbytes);
>> +   return memset(ckrealloc(NULL, nbytes), 0, nbytes);
>> }
>>
>> static void *
>> stalloc(size_t nbytes)
>> {
>> ...
>> g_stacknleft -= aligned;
>> +   memset(p, 0, nbytes);
>> return p;
>> }
>>
>> If this helps, then it's only a matter of finding a ckmalloc/stalloc
>> which really needs to be ckzalloc/stzalloc.
>> 
>
> Sorry Denys :(
> I've no access to my embedded system now and the comming 2+ weeks.
> Holidays ;-)
>
> Maybe a 16Mb RAM qemu could help nail down these problems, in absence of a
> low memory embeded system, as it seems one of the factors that provokes
> the errors may be low memory, but I'm just guessing.

What about initializing the memory to non-zero values? That should help 
to find uninitialized references.

static void *
ckmalloc(size_t nbytes)
{
-   return ckrealloc(NULL, nbytes);
+   return memset(ckrealloc(NULL, nbytes), 0xDC, nbytes);
}

static void *
stalloc(size_t nbytes)
{
...
g_stacknleft -= aligned;
+   memset(p, 0xDC, nbytes);
return p;
}


Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: [patch] - bbsplash applet

[Ralf Friedl]

Paul Fox wrote:
> you should replace ppm with just one image format -- a raw binary
> format that exactly matches the framebuffer format.
>
> your code that loads the fb should move into a separate utility
> that creates such a file.  then the raw splash images can be
> generated offline.  and your boot-time code could perhaps be
> reduced to "dd /dev/fb0".
>
> the code to generate the raw images can be in busybox, if you
> like, but it should be possible to build busybox with bbsplash,
> and without bbsplash_image_prepare, or whatever you call the raw
> file converter.  there may even already be a pnm utility to
> convert pnm images to raw rgb data, though i don't think there
> was, last time i looked -- i had to write my own pnmto565.c (which
> looked much like your drawimage() routine).
>   
The binary PPM format as almost as raw as it can get, while keeping 
information about the file size.
The conversion of binary PPM to raw rgb can normally be done with "tail 
-n +4", although a program "ppmtorgb3" is also available for files with 
unusual line breaks or bit depths.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: [patch] - bbsplash applet

[Ralf Friedl]

Paul Fox wrote:
> ralf wrote:
>  > Paul Fox wrote:
>  > > you should replace ppm with just one image format -- a raw binary
>  > > format that exactly matches the framebuffer format.
>  ...
>  > The binary PPM format as almost as raw as it can get, while keeping 
>  > information about the file size.
>
> i don't understand how a single PPM binary format can match various
> framebuffer depths.  (and -- i may be wrong on this -- don't framebuffers
> differ in pixel encoding, as well?  RGB vs. BGR, 565 vs. 556, etc?)
>   
PPM is normally RGB 888.
I assumed that is also the format of the frame buffer, as these days 
every graphics adapter should have enough memory for 24 bit.
>  > The conversion of binary PPM to raw rgb can normally be done with "tail 
>  > -n +4", although a program "ppmtorgb3" is also available for files with 
>  > unusual line breaks or bit depths.
>
> again, i don't understand.  ppmtorgb3 creates three files.  how
> does this help with writing a framebuffer?
I was wrong in my assumption about ppmtorgb3.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: Does lightweight hostname lookup applet exist in BB?

[Ralf Friedl]

JoSH Lehan wrote:
> Use "host" (or other command) to do a DNS query and resolve the
> hostname to an IP address.
> Feed that IP address to wget.
> Give the true hostname to wget as well, so that it can fill in the
> correct "Host:" header when doing the HTTP query.
> This lets you grab a URL without needing to install all the full NSS
> infrastructure on your local filesystem.
Why not just pass the hostname to wget? wget will resolve the hostname 
and use it in the HTTP header.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: How to upgrade glibc

[Ralf Friedl]

Christophe Osuna wrote:
> ~# cd /lib
> /lib# mount -o remount,rw /
> /lib# cp libc-2.5.so libc-2.5.so.new
> /lib# mv libc-2.5.so.new libc-2.5.so
> /lib# mount -o remount,ro /
> mount: mounting /dev/hda1 on / failed: Device or resource busy
> /lib#
>
> And after rebooting:
>
> ...
You should do a normal shutdown after that, not a failed remount to rw 
and after that a power cycle.

If you can't use a normal shutdown, your solution to keep the old file 
till the next reboot is probably necessary.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: lsattr operation not supported

[Ralf Friedl]

Kevin Holland wrote:
> I'm using busybox version 1.5.1 any idea why it says:
> lsattr zImage.elf
> lsattr: reading zImage.elf: Operation not supported
>   
Probably your filesystem and/or kernel is not configured to support it.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: busybox binaries hangs when trying to do "lstat"

[Ralf Friedl]

Sid Kapoor wrote:
> Hi all,
>
> The busybox 'ls' applet uses the kernel(system) calls such as lstat to
> locate mounted file system information. When a file system has been
> mounted from an NFS server and that server is temporarily unavailable,
> the calls 'lstat' uses may block in the kernel, resulting in the whole
> terminal in a hung-up status.
>
> I NFS mounted a folder at the mount point '/root/tmp/'. When the
> server was unavailable, and 'ls' was executed at /root, it tried to
> locate mounted file system's information. Eventually the system did
> not respond to the 'lstat' call and the system hanged. The final lines
> of the log of the strace command while executing 'ls' is as follows
> (full log file is attached with this mail) :
>
>
> lstat64("./tmp", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
> --- SIGHUP (Hangup) @ 0 (0) ---
> +++ killed by SIGHUP +++
>
> Can anyone tell the reason why is this happening? Is it a bug in
> busybox? Waiting for reply.
>   
The last line from strace shows that lstat succeeds.
You may consider the NFS option 'soft' for your mount.

The reason why busybox ls uses lstat is that is uses colored display of 
names by default, even with no options or environment variables set.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: busybox binaries hangs when trying to do "lstat"

[Ralf Friedl]

Denys Vlasenko wrote:
> Well, I don't understand what the original post implies. What ls
> should do instead? It has to obtain filename list SOMEHOW, no?
> It can be done only by asking the kernel to do readdir, [l]stat,
> etc.
In case of "ls" without options, readdir() without lstat would be 
enough, if busybox would not do colored output by default.
Coreutils "ls" does not lstat when it doesn't create colored output 
(which is default if stdout is not a tty), because the stat information 
is not used in that case..

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: busybox binaries hangs when trying to do "lstat"

[Ralf Friedl]

Denys Vlasenko wrote:
> On Friday 06 June 2008 22:13, Ralf Friedl wrote:
>   
>> In case of "ls" without options, readdir() without lstat would be 
>> enough, if busybox would not do colored output by default.
>> 
>
> Well, this is true only if you want a bare list of files.
> No coloring, no file type suffixes ("dir/"), no recursion (-R),
> no file info (-l).
>
> In other words, this is an optimization which is "nice to have"
> but in real world usage it won't happen to be used as often
> as one might want.
>   
I know, and I didn't want to imply that busybox should be changed. It 
was just a possible explanation why this might have been considered a 
busybox problem in the original message.

> Back to "NFS stall" question: readdir will stall as readily as lstat
> if the NFS server is down.
>   
I agree that the NFS server is the main problem and it's not a problem 
of busybox can solve.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: msh if not

[Ralf Friedl]

Roberto A. Foglietta wrote:
>  I would know how I can write this
>
>  if ! true; then echo ciao; fi
>
>  in msh's dialet because msh does not recognize ! (not)
You can use the else part:

if true; then :; else echo ciao; fi

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

[PATCH] Write pid-file in httpd

[Ralf Friedl]

This is a patch to create a pid-file when httpd is started.
A pid-file is useful to stop httpd or to send SIGHUP to reload the config.
The filename is passed as a parameter so that multiple instances can run 
at the same time, with different pid-files.

It would also be possible to make this configurable.

Regards
Ralf Friedl

--- networking/httpd.c
+++ networking/httpd.c
@@ -2270,6 +2270,7 @@
   USE_FEATURE_HTTPD_BASIC_AUTH(r_opt_realm ,)
   USE_FEATURE_HTTPD_AUTH_MD5(  m_opt_md5   ,)
   USE_FEATURE_HTTPD_SETUID(u_opt_setuid,)
+   p_opt_pidfile   ,
   p_opt_port  ,
   p_opt_inetd ,
   p_opt_foreground,
@@ -2281,6 +2282,7 @@
   OPT_REALM   = USE_FEATURE_HTTPD_BASIC_AUTH((1 << 
r_opt_realm )) + 0,
   OPT_MD5 = USE_FEATURE_HTTPD_AUTH_MD5(  (1 << 
m_opt_md5   )) + 0,
   OPT_SETUID  = USE_FEATURE_HTTPD_SETUID((1 << 
u_opt_setuid)) + 0,
+   OPT_PIDFILE = 1 << p_opt_pidfile,
   OPT_PORT= 1 << p_opt_port,
   OPT_INETD   = 1 << p_opt_inetd,
   OPT_FOREGROUND  = 1 << p_opt_foreground,
@@ -2294,6 +2296,7 @@
   int server_socket = server_socket; /* for gcc */
   unsigned opt;
   char *url_for_decode;
+   char *pidfilename;
   USE_FEATURE_HTTPD_ENCODE_URL_STR(const char *url_for_encode;)
   USE_FEATURE_HTTPD_SETUID(const char *s_ugid = NULL;)
   USE_FEATURE_HTTPD_SETUID(struct bb_uidgid_t ugid;)
@@ -2317,12 +2320,14 @@
   USE_FEATURE_HTTPD_BASIC_AUTH("r:")
   USE_FEATURE_HTTPD_AUTH_MD5("m:")
   USE_FEATURE_HTTPD_SETUID("u:")
+   "P:"
   "p:ifv",
   &configFile, &url_for_decode, &home_httpd
   USE_FEATURE_HTTPD_ENCODE_URL_STR(, &url_for_encode)
   USE_FEATURE_HTTPD_BASIC_AUTH(, &g_realm)
   USE_FEATURE_HTTPD_AUTH_MD5(, &pass)
   USE_FEATURE_HTTPD_SETUID(, &s_ugid)
+   , &pidfilename
   , &bind_addr_or_port
   , &verbose
   );
@@ -2403,6 +2408,8 @@
#if BB_MMU
   if (!(opt & OPT_FOREGROUND))
   bb_daemonize(0); /* don't change current directory */
+   if (opt & OPT_PIDFILE)
+   write_pidfile(pidfilename);
   mini_httpd(server_socket); /* never returns */
#else
   mini_httpd_nommu(server_socket, argc, argv); /* never returns */


___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: SENDMAIL: CC

[Ralf Friedl]

[EMAIL PROTECTED] wrote:
> My! Seems I discovered the source of your "do CC to the list" messages.
> BB sendmail, when used with -t option, just swallows Cc: headers.
>   
For completeness you should also add support for BCC.

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

[PATCH] Write pid-file in httpd

[Ralf Friedl]

Some time ago I sent this patch for httpd. Is there something wrong with it or 
was it simply ignored?

This is a patch to create a pid-file when httpd is started.
A pid-file is useful to stop httpd or to send SIGHUP to reload the config.
The filename is passed as a parameter so that multiple instances can run 
at the same time, with different pid-files.

It would also be possible to make this configurable.

Regards
Ralf Friedl

--- networking/httpd.c
+++ networking/httpd.c
@@ -2270,6 +2270,7 @@
  USE_FEATURE_HTTPD_BASIC_AUTH(r_opt_realm ,)
  USE_FEATURE_HTTPD_AUTH_MD5(  m_opt_md5   ,)
  USE_FEATURE_HTTPD_SETUID(u_opt_setuid,)
+   p_opt_pidfile   ,
  p_opt_port  ,
  p_opt_inetd ,
  p_opt_foreground,
@@ -2281,6 +2282,7 @@
  OPT_REALM   = USE_FEATURE_HTTPD_BASIC_AUTH((1 << 
r_opt_realm )) + 0,
  OPT_MD5 = USE_FEATURE_HTTPD_AUTH_MD5(  (1 << 
m_opt_md5   )) + 0,
  OPT_SETUID  = USE_FEATURE_HTTPD_SETUID((1 << 
u_opt_setuid)) + 0,
+   OPT_PIDFILE = 1 << p_opt_pidfile,
  OPT_PORT= 1 << p_opt_port,
  OPT_INETD   = 1 << p_opt_inetd,
  OPT_FOREGROUND  = 1 << p_opt_foreground,
@@ -2294,6 +2296,7 @@
  int server_socket = server_socket; /* for gcc */
  unsigned opt;
  char *url_for_decode;
+   char *pidfilename;
  USE_FEATURE_HTTPD_ENCODE_URL_STR(const char *url_for_encode;)
  USE_FEATURE_HTTPD_SETUID(const char *s_ugid = NULL;)
  USE_FEATURE_HTTPD_SETUID(struct bb_uidgid_t ugid;)
@@ -2317,12 +2320,14 @@
  USE_FEATURE_HTTPD_BASIC_AUTH("r:")
  USE_FEATURE_HTTPD_AUTH_MD5("m:")
  USE_FEATURE_HTTPD_SETUID("u:")
+   "P:"
  "p:ifv",
  &configFile, &url_for_decode, &home_httpd
  USE_FEATURE_HTTPD_ENCODE_URL_STR(, &url_for_encode)
  USE_FEATURE_HTTPD_BASIC_AUTH(, &g_realm)
  USE_FEATURE_HTTPD_AUTH_MD5(, &pass)
  USE_FEATURE_HTTPD_SETUID(, &s_ugid)
+   , &pidfilename
  , &bind_addr_or_port
  , &verbose
  );
@@ -2403,6 +2408,8 @@
#if BB_MMU
  if (!(opt & OPT_FOREGROUND))
  bb_daemonize(0); /* don't change current directory */
+   if (opt & OPT_PIDFILE)
+   write_pidfile(pidfilename);
  mini_httpd(server_socket); /* never returns */
#else
  mini_httpd_nommu(server_socket, argc, argv); /* never returns */



___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Re: [PATCH] Write pid-file in httpd

[Ralf Friedl]

Denys Vlasenko wrote:
> On Friday 18 July 2008 16:24, Ralf Friedl wrote:
>   
>> This is a patch to create a pid-file when httpd is started.
>> A pid-file is useful to stop httpd or to send SIGHUP to reload the config.
>> The filename is passed as a parameter so that multiple instances can run 
>> at the same time, with different pid-files.
>>
>> It would also be possible to make this configurable.
>> 
> I was sitting on it for some time. You know, I am not a big fan
> of pid files. I am a fan of djb's style of doing things,
> when filesystem is used as a fairly natural mapping to the
> set(s) of running processes. I am talking about runsv/runsvdir
> applets. Look at this gem:
>
> SIGNALS
> Syslogd reacts to a set of signals. You may easily send
> a signal to syslogd using the following:
> kill -SIGNAL `cat /var/run/syslogd.pid`
>
> Ok, I am not going to lecture everyone on "runsv way".
> But for god's sake, use "killall -SIGNAL syslogd" or, if your system
> has no killall (highly unlikely), "kill -SIGNAL `pidof syslogd`".
> This should work, right? And unlike pidfile, killall is immune to
> "stale pidfile" and "somehow two syslogd's were started, what now?!"
> problems.
>
> I remember that you propose having pifile's name specified as a param,
> which makes pidfile not as rediculous at /var/run/syslogd.pid.
>   
As I already mentioned, I want to run multiple instances of httpd at the 
same time with different configurations, so the approach with "killall" 
of "pidof" is not appropriate. Sending SUGHUP to all is possible, 
although not desirable, but with SIGTERM it is important to send it only 
to the right process.
> But still, would this work as well?
>
> #!/bin/sh
> echo $$ >pidfile
> exec httpd "$@"
>   
Using a wrapper Script should be possible, but it would probably need 
more bytes that creating the pid-file in httpd itself.
Also it would be necessary to run httpd in foreground mode, and the 
possibility of different pid-files. Something like this might work:

#!/bin/sh
echo $$ > "$1"
shift
exec httpd -f "$@"

Regards
Ralf Friedl
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox