kill builtin in ash might behave like killall

2017-03-03 Thread Emmanuel Deloget
Hello,

I'm using BB 1.26.2 with an heavily modified LEDE where I added
procps-ng and sysvinit. Given that setup, I'm using /usr/bin/kill from
props-ng and killall5 from sysvinit, meaning that I have the following
configuration:

# CONFIG_FEATURE_KILL_REMOVED is not set
CONFIG_FEATURE_KILL_DELAY=0
# CONFIG_RFKILL is not set
# CONFIG_KILL is not set
CONFIG_KILLALL=y
# CONFIG_KILLALL5 is not set
# CONFIG_PKILL is not set

The code in the kill.c applet is quite clear in this case:

...
#elif !ENABLE_KILL && ENABLE_KILLALL && !ENABLE_KILLALL5
# define killall  1
# define killall5 0
...

My problem is that I'm also using ash with job control enabled,
meaning that while I don't have the kill applet installed, I am still
using the kill.c code through the kill builtin.

You probably begin to see where I'm going: in this specific case, the
kill builtin does not work correctly, because it believes it should
behave as the killall command. And, of course, loads of scripts are
now unable to work correctly.

The obvious solution would be to disable CONFIG_KILLALL yet I also use
it from time to time on the command line, so I'd prefer to keep it.

For now, I'm doing a local patch to always check char3. This is
probably suboptimal yet I cannot see a better solution for this
specific issue. Does any of you have a better idea?

Best regards,

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


Re: kill builtin in ash might behave like killall

2017-03-03 Thread Emmanuel Deloget
Hello,

On Fri, Mar 3, 2017 at 1:44 PM, Tito  wrote:
>
>
> On 03/03/2017 01:01 PM, Emmanuel Deloget wrote:
>>
>> Hello,
>>
>> I'm using BB 1.26.2 with an heavily modified LEDE where I added
>> procps-ng and sysvinit. Given that setup, I'm using /usr/bin/kill from
>> props-ng and killall5 from sysvinit, meaning that I have the following
>> configuration:
>>
>> # CONFIG_FEATURE_KILL_REMOVED is not set
>> CONFIG_FEATURE_KILL_DELAY=0
>> # CONFIG_RFKILL is not set
>> # CONFIG_KILL is not set
>> CONFIG_KILLALL=y
>> # CONFIG_KILLALL5 is not set
>> # CONFIG_PKILL is not set
>>
>> The code in the kill.c applet is quite clear in this case:
>>
>> ...
>> #elif !ENABLE_KILL && ENABLE_KILLALL && !ENABLE_KILLALL5
>> # define killall  1
>> # define killall5 0
>> ...
>>
>> My problem is that I'm also using ash with job control enabled,
>> meaning that while I don't have the kill applet installed, I am still
>> using the kill.c code through the kill builtin.
>>
>> You probably begin to see where I'm going: in this specific case, the
>> kill builtin does not work correctly, because it believes it should
>> behave as the killall command. And, of course, loads of scripts are
>> now unable to work correctly.
>>
>> The obvious solution would be to disable CONFIG_KILLALL yet I also use
>> it from time to time on the command line, so I'd prefer to keep it.
>>
>> For now, I'm doing a local patch to always check char3. This is
>> probably suboptimal yet I cannot see a better solution for this
>> specific issue. Does any of you have a better idea?
>>
>> Best regards,
>>
>> -- Emmanuel Deloget
>
>
> Hi,
> you could disable CONFIG_KILLALL in your main busybox binary
> and create a second binary that contains only the killall
> applet and rename it killall, if space is not a problem.
>
> Ciao,
> Tito

Thanks Tito, but unfortunatly that would mean to change the way LEDE
packages are built (which is way harder than applying my small,
miserable patch on top of BB source).

Space is not much an issue, yet having a full BB binary for killall is
a bit, well, overkill. It would be far simpler to add psmisc as a LEDE
package. But the best solution would still be to fix the kill applet,
since the configuration I use is a valid configuration (and the kill
builtin should never behave as killall, whatever the configuration
is).

Best regards,

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


Re: kill builtin in ash might behave like killall

2017-03-03 Thread Emmanuel Deloget
Hi,

On Fri, Mar 3, 2017 at 3:17 PM, Kang-Che Sung  wrote:
> On 03/03/2017 01:01 PM, Emmanuel Deloget wrote:
>>
>> Hello,
>>
>> I'm using BB 1.26.2 with an heavily modified LEDE where I added
>> procps-ng and sysvinit. Given that setup, I'm using /usr/bin/kill from
>> props-ng and killall5 from sysvinit, meaning that I have the following
>> configuration:
>>
>> # CONFIG_FEATURE_KILL_REMOVED is not set
>> CONFIG_FEATURE_KILL_DELAY=0
>> # CONFIG_RFKILL is not set
>> # CONFIG_KILL is not set
>> CONFIG_KILLALL=y
>> # CONFIG_KILLALL5 is not set
>> # CONFIG_PKILL is not set
>>
>> The code in the kill.c applet is quite clear in this case:
>>
>> ...
>> #elif !ENABLE_KILL && ENABLE_KILLALL && !ENABLE_KILLALL5
>> # define killall  1
>> # define killall5 0
>> ...
>>
>> My problem is that I'm also using ash with job control enabled,
>> meaning that while I don't have the kill applet installed, I am still
>> using the kill.c code through the kill builtin.
>>
>> You probably begin to see where I'm going: in this specific case, the
>> kill builtin does not work correctly, because it believes it should
>> behave as the killall command. And, of course, loads of scripts are
>> now unable to work correctly.
>>
>> The obvious solution would be to disable CONFIG_KILLALL yet I also use
>> it from time to time on the command line, so I'd prefer to keep it.
>>
>> For now, I'm doing a local patch to always check char3. This is
>> probably suboptimal yet I cannot see a better solution for this
>> specific issue. Does any of you have a better idea?
>>
>> Best regards,
>>
>> -- Emmanuel Deloget
>
> I think I have proposed a patch that fixes this and it's on master now.
>
> Here, if you can adapt it and apply it for yourself:
> https://git.busybox.net/busybox/commit/?id=61a91af63dbc91f85058efda5c542dfc859ab1be

I missed it. And yeah, it does exactly what I want to do.

Thanks for your help!

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


Re: [PATCH] add lsscsi implementation

2017-04-07 Thread Emmanuel Deloget
Hi Markus,


On Fri, Apr 7, 2017 at 9:52 PM, Markus Gothe  wrote:
>
> Ahhh, for some reason it disappeared…
>
>
>
> On 07 Apr 2017, at 21:01 , Tito  wrote:
>
> > Hi,
> > you forgot to attach the patch?
> >
> > Ciao,
> > Tito
> >
> > On 04/07/2017 07:12 PM, Markus Gothe wrote:
> >> Please see attached patch. This is a minimal lsscsi which is useful for
> >> getting information about USB-disks and other devices populating the
> >> SCSI bus in Linux.
>

It seems the config: comments are incorrectly refering to lsusb
instead of lsscsi.

>
> >> BR,
> >> //Markus - The panama-hat hacker
> >>

Best regards,

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

Re: [PATCH 1/1] makedevs: set path size to match linux

2017-06-26 Thread Emmanuel Deloget
Hello,

On Mon, Jun 26, 2017 at 11:23 PM, Matthew Weber <
matthew.we...@rockwellcollins.com> wrote:

> Baruch,
>
> On Mon, Jun 26, 2017 at 3:55 PM, Baruch Siach  wrote:
> > Hi Jared,
> >
> > On Mon, Jun 26, 2017 at 03:33:09PM -0500, Matt Weber wrote:
> >> From: Jared Bents 
> >>
> >> Update to increase the pathname limit to the
> >> linux limit of 4096 characters.
> >>
> >> Similar patch:
> >> https://patchwork.openembedded.org/patch/131475/
> >>
> >> Signed-off-by: Jared Bents 
> >> Signed-off-by: Matt Weber 
> >> ---
> >>  miscutils/makedevs.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/miscutils/makedevs.c b/miscutils/makedevs.c
> >> index 9e7ca34..0049edb 100644
> >> --- a/miscutils/makedevs.c
> >> +++ b/miscutils/makedevs.c
> >> @@ -208,7 +208,7 @@ int makedevs_main(int argc UNUSED_PARAM, char
> **argv)
> >>   unsigned count = 0;
> >>   unsigned increment = 0;
> >>   unsigned start = 0;
> >> - char name[41];
> >> + char name[4096];
> >
> > Why not use PATH_MAX here?
>
> Agree, that would be cleaner.  Will submit v2 after some testing.
>
> That still leaves a hardcoded value in the sscanf  of 4095
> should I add a comment to the affect we're assuming PATH_MAX is at
> least 4096?  Maybe a check is also needed?
>

​Alternatively you may use the m modifier, when implemented, to
auto-allocate the name pointer. ​This way you don't have to hardcode
anything in the sscanf(), you let the library for the job for you. Later,
you can check the string length.

Such a change would induce an allocation, a free but will also reduce stack
usage.




>
> Matt
>
>
​Best regards,

-- Emmanuel Deloget​
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Silence misguided GCC warning about alignment issues

2017-08-07 Thread Emmanuel Deloget
Hello,


On Mon, Aug 7, 2017 at 12:25 PM, Lauri Kasanen  wrote:
> On Mon, 7 Aug 2017 12:09:45 +0200 (CEST)
> Johannes Schindelin  wrote:
>> Meaning: GCC gets this all wrong and should not complain. So let's force
>> it to be quiet for a couple of lines.
>
> I believe this is both wrong and dangerous. If gcc warns about it, it
> will also very likely try to mis-optimize the same case.
>
> - Lauri

I tried to carefully read the code, and I don't see where the aliasing
issue migh be.
* buf is an uin8_t []
* the code either use it as is, or promote it to const uint8_t *, or
cast it to (const) char *.

All of these types are compatible w.r.t. aliasing (and the C99
standard). I may have missed something though and my eyes might be
rusted.

Now, it seems that some might have seen a regression in GCC with
respect to aliasing rules [1][2]. This might be an instance of the bug
(according to the bug report, it's no longer reproducible in GCC
trunk).

Anyway, it might be a good idea to avoid unsetting the warning for one
buggy GCC version :)

Best regards,

-- Emmanuel Deloget

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80593
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80633
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] Silence misguided GCC warning about alignment issues

2017-08-07 Thread Emmanuel Deloget
On Mon, Aug 7, 2017 at 9:46 PM, Johannes Schindelin <
johannes.schinde...@gmx.de> wrote:

> Hi Denys,
>
> On Mon, 7 Aug 2017, Denys Vlasenko wrote:
>
> > On Mon, Aug 7, 2017 at 12:09 PM, Johannes Schindelin
> >  wrote:
> > > When compiling xz_dec_stream.c with GCC 7.1.0, it complains thusly:
> > >
> > > In function 'dec_stream_footer':
> > > error: dereferencing type-punned pointer will break
> strict-aliasing
> > >   rules [-Werror=strict-aliasing]
> > >if (xz_crc32(s->temp.buf + 4, 6, 0) !=
> get_le32(s->temp.buf))
> > >^~
> > >
> > > The thing is, the `buf` field was put just after two fields of type
> > > size_t for the express purpose of avoiding alignment issues, as per the
> > > comment above the `temp` struct.
> > >
> > > Meaning: GCC gets this all wrong and should not complain. So let's
> force
> > > it to be quiet for a couple of lines.
> >
> > xz_crc32 is:
> >
> > xz_crc32(const uint8_t *buf, size_t size, uint32_t crc)
> >
> > I think gcc is not complaining about xz_crc32,
> > it complains about get_le32!
> >
> > Can you test this theory by splitting that statement in two?
>
> I, too, thought at first that it is clearly about get_le32(), but GCC was
> really adamant that it was the xz_crc32() call.
>
> And a little further investigation suggests that GCC really meant
> xz_crc32(), which is defined as a static function in
> archival/libarchive/decompress_unxz.c that gets inlined by GCC because it
> simply hands off to crc32_block_endian0() which in turn takes an
> `uint32_t *` as first parameter. And that's where the type-punning is
> broken.
>
>
​This would be quite weird - the static function is by definition local to
the compilation unit, and the compiler cannot know about ​the content of
decompress_unxz.c from xz_dec_stream.c.

BR,

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

Re: [PATCH] Silence misguided GCC warning about alignment issues

2017-08-07 Thread Emmanuel Deloget
On Mon, Aug 7, 2017 at 10:37 PM, Emmanuel Deloget  wrote:
>
>
>
> On Mon, Aug 7, 2017 at 9:46 PM, Johannes Schindelin 
>  wrote:
>>
>> Hi Denys,
>>
>> On Mon, 7 Aug 2017, Denys Vlasenko wrote:
>>
>> > On Mon, Aug 7, 2017 at 12:09 PM, Johannes Schindelin
>> >  wrote:
>> > > When compiling xz_dec_stream.c with GCC 7.1.0, it complains thusly:
>> > >
>> > > In function 'dec_stream_footer':
>> > > error: dereferencing type-punned pointer will break 
>> > > strict-aliasing
>> > >   rules [-Werror=strict-aliasing]
>> > >if (xz_crc32(s->temp.buf + 4, 6, 0) != get_le32(s->temp.buf))
>> > >^~
>> > >
>> > > The thing is, the `buf` field was put just after two fields of type
>> > > size_t for the express purpose of avoiding alignment issues, as per the
>> > > comment above the `temp` struct.
>> > >
>> > > Meaning: GCC gets this all wrong and should not complain. So let's force
>> > > it to be quiet for a couple of lines.
>> >
>> > xz_crc32 is:
>> >
>> > xz_crc32(const uint8_t *buf, size_t size, uint32_t crc)
>> >
>> > I think gcc is not complaining about xz_crc32,
>> > it complains about get_le32!
>> >
>> > Can you test this theory by splitting that statement in two?
>>
>> I, too, thought at first that it is clearly about get_le32(), but GCC was
>> really adamant that it was the xz_crc32() call.
>>
>> And a little further investigation suggests that GCC really meant
>> xz_crc32(), which is defined as a static function in
>> archival/libarchive/decompress_unxz.c that gets inlined by GCC because it
>> simply hands off to crc32_block_endian0() which in turn takes an
>> `uint32_t *` as first parameter. And that's where the type-punning is
>> broken.
>>
>
> This would be quite weird - the static function is by definition local to the 
> compilation unit, and the compiler cannot know about the content of 
> decompress_unxz.c from xz_dec_stream.c.

My bad. It's the other way round -- sorry for the noise.
decompress_unxz.c includes xz_dec_stream.c (sometimes, the Busybox
code is a bit hard to follow :))

And yes, its seems that the get_le32() macro in xz_private.h is a bit
illegal with respect to strict aliasing, as it casts a uint8_t * into
a const uint32_t *.

>
> BR,
>
> -- Emmanuel Deloget
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] Silence misguided GCC warning about alignment issues

2017-08-08 Thread Emmanuel Deloget
Hi Johannes

On Tue, Aug 8, 2017 at 1:19 PM, Johannes Schindelin <
johannes.schinde...@gmx.de> wrote:

> Hi Emmanuel,
>
> On Mon, 7 Aug 2017, Emmanuel Deloget wrote:
>
> > And yes, its seems that the get_le32() macro in xz_private.h is a bit
> > illegal with respect to strict aliasing, as it casts a uint8_t * into a
> > const uint32_t *.
>
> It would seem so.
>
> Until you realize that it is used only on s->temp.buf (or `s->temp.buf + 4
> * `), and that the `buf` field was carefully placed after two
> fields of type `size_t` (i.e. naturally aligned *at least* to a 32-bit
> boundary).
>
> If you are worried about that magic, you can even mark the `buf` field
> using ALIGN4. But that *still* does not fix GCC's compiler warning. What
> fixes it is my workaround of defining a static ALWAYS_INLINE function.
>
>
​No, I'm not worried by the magic :)

Yet the problem is not the aliasing per see, it's the way the compiler
handles it and what it means to him.

Strict aliasing has been defined as a way to allow the compiler to do some
clever code optimization (mostly reordering). If the compiler knows that
two memory areas are distinct, there is no R/W dependency on these areas
and it can reorder the code as he wants. But when aliasing comes into play,
such optimizations should be forbidden since an undetected R/W dependency
might exists. Yet, since the -fstrict-aliasing is given to him, we
explicitely tell the compiler to force strict aliasing rules and the
compiler ​still apply them -- ditching any unproved R/W dependency in the
process.

Needless to say, this can be the source of hard-to-catch bugs :)

Consider this code:

uint32_t k = 0;
uint8_t *p = (uint8_t *)&k;

k = 0x;
if (*p == 0xff)
   do_some_magic_stuff();

Without strict aliasing, the compiler must order the code so that *p is
indeed 0xff when the if condition is evaluated because it cannot be sure
that there is a R/W dependency between k and p. With forced strict
aliasing, &k and p are assumed to not point to the same memory area, thus
there cannot be any R/W dependency, so changing k is not supposed to have
any effect on the *p test. As a consequence, the compiler is free to
reorder the code and for exemple do the following:

if (*p = 0xff)
  do_some_magic_stuff();
k = 0x;

Which, of course, will not work as expected.

Thus, this aliasing problem may have other unintended consequences.
Silencing the warning will hide a possible bug, and this is not a good
thing. It would be far better to fix the aliasing issue correctly.
Similarly, the fact that ALWAY_INLINE seems to hide the warning looks more
like a GCC bug to me than the expected behavior, as ALWAYS_INLINE does not
change the way the compiler plays with the aliasing rules.

Something along the line of (not tested):

#define get_le32(p) \
  ({ \
union { const uint32_t *x32; const uint8_t *x8; } v = { .x8 = (p), }; \
le32_to_cpup(v.x32); \
  })

​(for the occurence at xz_private.h@24)

Maybe it's even better to create an aliastype macro looking like:

#define aliasvalue(type, value) \
  ({ \
union { \
  type *aliased; \
  typeof(value) ​*original; \
} v = { \
  .original = &(value), \
}; \
v.aliased; \
  })

​And then:

#define get_le32(p) le32_to_cpup(aliasvalue(uint32_t, (p)))

Or

#define get_le32(p) (*(aliasvalue(uint32_t,(p

I'm not even sure the compiler is not able to fully optimize out this kind
of cod since it will look like a move from one register to another register
to him (my own tests seems to show that the compiler emits optimized code
in this situation).
​
The aliasvalue() macro above is probably not perfect (it might run ok when
p in a scalar or an array of scalar but will break if p is a pointer; not
to mention its horrible name...) so more though might be needed to come up
with a correct version.



> Denys' fix works around the problem, alright, but it also makes the code
> slower. And it is *not* necessary to make the code slower.
>
> Ciao,
> Johannes
>

​Best regards,

-- Emmanuel Deloget​
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: bbox: musl versus uclibc

2017-08-14 Thread Emmanuel Deloget
On Mon, Aug 14, 2017 at 7:43 PM, Denys Vlasenko
 wrote:
> Only a few options did not build:
> EXTRA_COMPAT and FEATURE_VI_REGEX_SEARCH
> failed because they need GNU regexp extensions.

I have a patch somewhere that enable parts of VI_REGEX_SEARCH (it only
does forward regex search while the GNU counterpart implements both
forward and backward regex search). I can submit that if the community
is interested.

BR,

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


Re: Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

2018-01-03 Thread Emmanuel Deloget
Hello,

On Wed, Jan 3, 2018 at 11:04 PM, Markus Gothe 
wrote:

> Can you run strace on a process invoked from the inittab? I surely have
> issues with doing that.
>

​This is definitely possible, but thou shall not forget -ff to follow forks
(and prepare for an awfull lot of output, so maybe you should redirect that
in some log file). ​



>
> As far as killing the processes it doesnt matter which signal you use;
> processes will become zombified forever without the patch.
>


​I, too, don't get how runsvdir children can be zombified if runsvdir is
killed. ​When runsvdir dies, the children are reparented to /bin/init which
is, AFAIK, reaping them.

I won't disagree with you (you say you have zombies all over the place) but
maybe the problem - and the right fix - lies somewhere else.

Best regards,

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


Re: Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

2018-01-04 Thread Emmanuel Deloget
Hello,

On Thu, Jan 4, 2018 at 9:55 PM, Markus Gothe 
wrote:

> Well, just to be sure we all talk about the same children that needs to be
> reaped are those of runsvdir, the bug manifests without using the inittab,
> surely init will reap the dead process of runsvdir but not the children
> themselves of runsvdir.
>

Any process of pid 1 is expected to be the ultimate child reaper, so once
runsvdir dies its children are bound to be reaped by init. That's how unix
has worked for decades :)

​BR,

-- Emmanuel Deloget​
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [**EXTERNAL**] Re: [PATCH] Fix runsvdir so it reaps children and avoid zombi processes when killed.

2018-01-04 Thread Emmanuel Deloget
Hello,

On Thu, Jan 4, 2018 at 6:53 PM, Cathey, Jim  wrote:

> >Unconditionally sleeping when signals may arrive is a no-no.
>
> It used to be that sleep(3) was interruptible.  Is this no longer the
> case?  Who broke libc, and when?
>


* sleep() in musl is implemented using nanosleep() [1].
* ucLibc-ng is using nanosleep() too [2], and implement a workaround for an
old linux bug (it seems that this workaround is not needed)
* ​and surpris, sleep() in the glibc is using... nanosleep() [3]

Now, sleep() shall be interrupted by any signal who either has a handler in
the thread or terminates the program [A]. The sames goes for nanosleep()
[B].

So when you install a SIGCHLD handler, the signal will kill sleep(). Now,
SIGCHLD is not catched by default (man 7 signal says the default is set to
"ignore"). So while sleep() is still interruptible, it won't be if the
delivered signal is SIGCHLD - unless you install a handler for this signal.
Since busybox' init does not do that, then no, sleep() will not be
interrupted in that case (and you face the problem Laurent describes).


>
> -- Jim
>
>
​Best regards,

-- Emmanuel Deloget​

​[1] https://git.musl-libc.org/cgit/musl/tree/src/unistd/sleep.c​
[2]
https://cgit.openadk.org/cgi/cgit/uclibc-ng.git/tree/libc/unistd/sleep.c#n74
[3] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/posix/sleep.c

[A] http://pubs.opengroup.org/onlinepubs/9699919799/functions/sleep.html
​[B]
http://pubs.opengroup.org/onlinepubs/9699919799/functions/nanosleep.html​
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] ash: make bash_source command search current directory after PATH

2018-01-29 Thread Emmanuel Deloget
Hello,

On Mon, Jan 29, 2018 at 4:42 PM, Paul Otto  wrote:

>
>
>
>> > Making bash "source" behavior non-standard had nothing useful in it.
>>
>> "source" is already non-standard and not specified in POSIX.  so simply by
>> using it, your script is not POSIX compliant.
>
>
> That is why, incidentally, I wrote my proposed contribution the way I did
> initially. In my view, while BASH treats "source" and "." the same, POSIX
> doesn't allow for "source" so why not have "source" hold to the BASH
> standard and "." hold to the POSIX standard? I definitely caved too quickly
> on that point, and wound up with my contribution being swallowed up into a
> patch that did the exact opposite of my intent.  ¯\_(ツ)_/¯
>
>
​This is probably due to the fact that many (for instance, I) expect . and
source to behave the same way :) Having them behaving differently would
somewhat violate the principle of least surprise :)

BR,

-- Emmanuel Deloget​
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: printf can't handle the '+' char in front of an integer

2018-10-31 Thread Emmanuel Deloget
Hello,

Just my two cents here.

On Wed, Oct 31, 2018 at 12:34 PM Bernhard Reutner-Fischer
 wrote:
>
> >
> > 64-bit double's mantissa is only 53 bits...
> >
> > With "long double" shenanigans, I managed to "improve" this to:
> >
> > $ ./busybox printf "%f\n" 18446744073709550592 18446744073709553665
> > 18446744073709550591.50
> > 18446744073709553663.50
> >
> > at this cost:
> >
> > function old new   delta
> > printf_main  909 976 +67
> > strtold-  19 +19
> > print_direc  457 475 +18
> > conv_strtod   54  61  +7
> > --
> > (add/remove: 3/0 grow/shrink: 3/0 up/down: 130/0) Total: 111 
> > bytes
> >
> > See attached. Do you think it's worth it?
>
> I don't know.
> long double can be very slow (emulated) and complex to support, which
> might not be of concern here though.
> But first and foremost nobody complained yet AFAIK.
> Maybe keep it as is? Actively reject values too large for plain double
> in printf(1) for now?
> dunno.

Maybe hide the float/double/long double behind a config ? Some people
may want printf to support long values, while some other (like me)
does not need it.

Best regards,

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


Re: Regarding [PATCH] sysklogd: add -Z option to adjust message timezones

2020-02-04 Thread Emmanuel Deloget
Hi Venkat,

But i didn't get where *timezone *is defined and what it is contained, Can
> you help us where it is defined and what it is contained.
>

timezone is defined in time.h and is documented in man 3 timezone.

Best regards,

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


Re: [PATCH] libbb/last_char_is: rewrite for smaller and faster code

2020-07-04 Thread Emmanuel Deloget
Hello,

On Fri, Jul 3, 2020 at 9:15 PM Tito  wrote:
>
>
>
> On 7/3/20 6:13 PM, Xabier Oneca -- xOneca wrote:
> > Hi all,
> >
> >>> On Fri, 2020-07-03 at 08:12 +0200, Tito wrote:
> >>> [...]
> >>>> Improved version:
> >>>>
> >>>> char* last_char_is(const char *s, int c)  {
> >>>>  if (!s || !*s) return NULL;
> >>>>  while (*(s + 1))s++;
> >>>>  return (c == *s) ? (char *)s : NULL;
> >>>> }
> >
> > Let me "improve" the Tito's -11 bytes, probably introducing some subtle bug 
> > :)
> >
> > char* FAST_FUNC last_char_is(const char *s, int c)
> > {
> > if (!s || !*s) return NULL;
> > while (*(++s));
> > return (*(--s) == c) ? s : NULL;
> > }
> >
> > function old new   delta
> > last_char_is  58  42 -16
> > --
> > (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-16) Total: -16 
> > bytes
> >textdata bss dec hex filename
> >  95644942351904  962588   eb01c busybox_old
> >  95643342351904  962572   eb00c busybox_unstripped
> >
> > Also, there's a warning compiling this function: return discards
> > ‘const’ qualifier from pointer target type
>
> Hi,
> cast it?
>
> char* FAST_FUNC last_char_is(const char *s, int c)
> {
>  if (!s || !*s) return NULL;
>  while (*(++s));
>  return (*(--s) == c) ? (char *)s : NULL;
> }
>

In favor of const-correctness, wouldn't it be better to return a const
char*? It's quite strange to promote a string to const and to a part
of it that is non-const (not to mention that if you feed the function
with a real const char* then the cast is not a good thing to do).

>
> > Fun to play with you.
> >
> > Cheers,
> >
> > Xabier Oneca_,,_

Best regards,

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


Re: [PATCH 4/5] httpd_indexcgi.c: Remove new lines in HTML

2020-08-05 Thread Emmanuel Deloget
On Wed, Aug 5, 2020 at 2:40 PM Tim Tassonis  wrote:
> On 7/31/20 9:51 PM, Sergey Ponomarev wrote:
> > Yes, it looks worse but you can see the resulting DOM tree instead
> > (elements/inspector). TBH as a web developer I almost never looked into
> > raw source and always watch the DOM tree.
>
> Not that I care terribly about this, but personally the newlines add
> very little in size, but a whole lot in
> readability/grep-ability/script-ability.
>
> The whole point of text formats generally are their readability without
> fancy tools such as web inspector, and process-ability with
> line-oriented tools such as sed/grep/head/tail/..., so removing the
> newlines is clearly a huge step backwards, for very little gain in size.

To add my two cents here, I'm with Tim and Denys here. I would prefer to
not trade readability and scriptability of the generated HTML for a few
bytes.

Of course, you're free to ignore my opinion :)

Best regards,

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


Re: [PATCH] chrt: support for musl C library

2020-11-10 Thread Emmanuel Deloget
Hello, 

- Mail original -
> On Fri, Sep 11, 2020 at 6:18 PM Christian Eggers 
> wrote:
> > musl "implements" several sched_xxx() functions by returning
> > ENOSYS. As
> > an alternative, either pthread_(g|s)etschedparam() or direct
> > syscalls
> > can be used.
> >
> > References:
> > https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/commit/schedutils/chrt.c?id=fcc3078754291d2f5121797eb91b364f8e24b2f1
> > References:
> > http://git.musl-libc.org/cgit/musl/commit/src/sched/sched_setscheduler.c?id=1e21e78bf7a5c24c217446d8760be7b7188711c2
> > Signed-off-by: Christian Eggers 
> > ---
> >  util-linux/chrt.c | 31 +--
> >  1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/util-linux/chrt.c b/util-linux/chrt.c
> > index 4dd78dabf..e465d7cec 100644
> > --- a/util-linux/chrt.c
> > +++ b/util-linux/chrt.c
> > @@ -34,6 +34,9 @@
> >  //usage:   "You need CAP_SYS_NICE privileges to set scheduling
> >  attributes of a process"
> >
> >  #include 
> > +#if defined (__linux__)
> > +# include 
> > +#endif
> >  #include "libbb.h"
> >  #ifndef SCHED_IDLE
> >  # define SCHED_IDLE 5
> > @@ -85,6 +88,7 @@ int chrt_main(int argc UNUSED_PARAM, char **argv)
> > char *priority = priority; /* for compiler */
> > const char *current_new;
> > int policy = SCHED_RR;
> > +   int ret;
> >
> > opt = getopt32(argv, "^"
> > "+" "mprfobi"
> > @@ -132,7 +136,15 @@ int chrt_main(int argc UNUSED_PARAM, char
> > **argv)
> > if (opt & OPT_p) {
> > int pol;
> >   print_rt_info:
> > +#if defined (__linux__) && defined(SYS_sched_getscheduler)
> > +   /* musl libc returns ENOSYS for its
> > sched_getscheduler library
> > +* function, because the sched_getscheduler Linux
> > kernel system call
> > +* does not conform to Posix; so we use the system
> > call directly
> > +*/
> > +   pol = syscall(SYS_sched_getscheduler, pid);
> > +#else
> > pol = sched_getscheduler(pid);
> > +#endif
> > if (pol < 0)
> > bb_perror_msg_and_die("can't %cet pid %u's
> > policy", 'g', (int)pid);
> >  #ifdef SCHED_RESET_ON_FORK
> > @@ -149,7 +161,12 @@ int chrt_main(int argc UNUSED_PARAM, char
> > **argv)
> > printf("pid %u's %s scheduling policy: SCHED_%s\n",
> > pid, current_new, policy_name(pol)
> > );
> > -   if (sched_getparam(pid, &sp))
> > +#if defined (__linux__) && defined(SYS_sched_getparam)
> > +   ret = syscall(SYS_sched_getparam, pid, &sp);
> > +#else
> > +   ret = sched_getparam(pid, &sp);
> > +#endif
> > +   if (ret)
> > bb_perror_msg_and_die("can't get pid %u's
> > attributes", (int)pid);
> > printf("pid %u's %s scheduling priority: %d\n",
> > (int)pid, current_new, sp.sched_priority
> > @@ -168,7 +185,17 @@ int chrt_main(int argc UNUSED_PARAM, char
> > **argv)
> > sched_get_priority_min(policy),
> > sched_get_priority_max(policy)
> > );
> >
> > -   if (sched_setscheduler(pid, policy, &sp) < 0)
> > +#if defined (__linux__) && defined(SYS_sched_setscheduler)
> > +   /* musl libc returns ENOSYS for its sched_setscheduler
> > library
> > +* function, because the sched_setscheduler Linux kernel
> > system call
> > +* does not conform to Posix; so we use the system call
> > directly
> > +*/
> > +   ret = syscall(SYS_sched_setscheduler, pid, policy, &sp);
> > +#else
> > +   ret = sched_setscheduler(pid, policy, &sp);
> > +#endif
> > +
> > +   if (ret < 0)
> > bb_perror_msg_and_die("can't %cet pid %u's policy",
> > 's', (int)pid);
> >
> > if (!argv[0]) /* "-p PRIO PID [...]" */
> 
> 
> I looked at it several times, but this looks not so good.
> These direct syscalls are less maintainable.
> 
> OTOH... by the looks of it, there is no alternative?

Maybe define a function or even a static inline that does the job? That should 
not cause any issue and it should be easier to maintain (not to mention that it 
could be added to libbb if needed in order to be reused in other places).

On a side note, instead of checking for __linux__ only, one can also check for 
!__GLIBC__ as well in order to limit the use case to libraries that are not the 
glibc (yes, I link busybox with the glibc, which more or less defeat the 
purpose of busybox).

#if defined(__linux__) && defined(SYS_sched_setscheduler) && !defined(__GLIBC__)

> I'll probably apply it.

Best regards, 

-- Emmanuel Deloget

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


Re: six patches from tinycore linux

2021-08-12 Thread Emmanuel Deloget
Hello,

On Thu, Aug 12, 2021 at 4:01 PM Ron Yorston  wrote:
>
> Roberto A. Foglietta wrote:
> >Hi Ron,
> >
> > what's about the other five?
>

* busybox-1.33.0_skip-loop-control.patch makes get_free_loop() behave
as if there was no free loop devices (unconditionnaly). It would help
to have some background info here, as it does not seem to fix a bug.

* busybox-1.29.3_root_path.patch adds /usr/local/bin to the list of
default PATH ; the whole value can be configured  by setting
BB_ADDITIONAL_PATH so I'm not sure there is much value here.

* busybox-1.27.1-wget-make-default-timeout-configurable.patch does
exactly what it says it does - it makes the default wget timeout a
configurable value. I agree that the default value of 900 seconds
might be a bit surprising. A value in [30, 120] might have been
better. I'm not sure whether the good thing to do is to make the value
configurable or provide a saner default value (15 minutes? that's a
bit long...). Is there a rationale for this very, very long timeout?

* busybox-1.33.0_modprobe.patch checks whether the module is an ELF
bianry before sending it to the kernel. Since the checks that the
kernel performs are more comprehensive, I don't see much value in
adding a simple check of the ELF header here -- not to mention that
the code is duplicated.

* busybox-1.33.0_tc_depmod.patch : it *seems* that this one solves a
bug but I cannot wrap my head around it. More particularly, I'm
wondering if this is a tinycore thing (having modules hidden behind
symlinks) or if this is a real issue.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: shell function name length, question

2021-08-14 Thread Emmanuel Deloget
Hello,

On Sat, Aug 14, 2021 at 12:19 PM Roberto A. Foglietta
 wrote:
>
> Hi all,
>
>  how long could be a shell function name?
>
>  In my last patch, I used 256 characters because: "On Linux: The maximum 
> length for a file name is 255 bytes". So, I have extended the concept to a 
> function name but it is not the same.
>
>  Thank you,

According to the discussion in [1], both bash and NETBSD sh can be
used with identifiers that are longer than 1000 characters. Bash seems
to not have any limit on identifier names. The POSIX sh standard
itself does not define anything related to the length of any
identifier (be it a variable name or a function name).

(I would totally accept shorter identifiers ; mine rarely goes larger
than 30-40 bytes, and these are the longest ; yet it's not difficult
to image a processor that could create longer identifier names).

[1] https://lists.gnu.org/archive/html/bug-bash/2018-11/msg00069.html
[2] https://pubs.opengroup.org/onlinepubs/9699919799/

> --
> Roberto A. Foglietta
> +39.349.33.30.697

Best regards,

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


Re: [PATCH] ash: fix command name of shell scripts

2021-10-11 Thread Emmanuel Deloget
Hello,

This is a real issue (one I didn't report and fixed with a patch of my
own, as I was not sure this was not caused by another patch of mine).
But if I'm not the only one with the problem then it might not be my
own fault :)

The problem lies in appletlib.c, at the end of main() -- when
(ENABLE_FEATURE_SH_STANDALONE || ENABLE_FEATURE_PREFER_APPLETS ||
!BB_MMU) is true. In this case, we force comm to be the applet name -
and in this case, the applet name is sh which is not what we want to
get.

The way I fix the problem is quite un-sophisticated (so don't apply as is).

(patch as attachment, because, well, gmail).

It workds well enough for my own usage, but I bet it can be enhanced
and corrected. The idea is to replace the comm by the applet name only
if the comm is "exe" (which we want to avoid). For unknown reasons it
seems to do the trick although I'm not sure why it does ; maybe the
condition is not correct and we are not in the case where we always
get comm == "exe". I did not investigate that much.

Best regards,

-- Emmanuel Deloget


Le lun. 11 oct. 2021 à 14:24,  a écrit :
>
> I forgot to mention that the issue isn't triggered by the default
> busybox configuration.
> I noticed the issue in OpenWrt which uses
> CONFIG_FEATURE_PREFER_APPLETS=y by default,
> and that option triggers the issue.
>
> Config.in says it's an experimental option. Does it mean it isn't
> recommended, and that
> OpenWrt maybe shouldn't enable it by default?
>
> /Mikael
>
> On 2021-10-11 02:58, Denys Vlasenko wrote:
> > On Sun, Oct 10, 2021 at 11:09 PM  wrote:
> >> Hello,
> >>
> >> when executing a script with ash from busybox in the shebang then the 
> >> command name will contain "sh" instead of the name of the script as 
> >> expected.
> > Can't reproduce: I see script name in comm field, not "sh" or "ash":
> >
> > $ cat comm.tests
> > {
> > echo "#!$THIS_SH"
> > echo 'procdir=/proc/$$'
> > echo 'echo "  /proc/N/exe:  $(basename $(readlink $procdir/exe))"'
> > echo 'echo "  /proc/N/comm: $(cat $procdir/comm)"'
> > } >SCRIPT.sh
> > chmod 755 SCRIPT.sh
> > echo ./SCRIPT.sh:
> > ./SCRIPT.sh
> > echo "exec ./SCRIPT.sh:"
> > (exec ./SCRIPT.sh)
> > echo sh ./SCRIPT.sh:
> > $THIS_SH ./SCRIPT.sh
> > rm SCRIPT.sh
> >
> > $ THIS_SH=/bin/bash bash comm.tests
> > ./SCRIPT.sh:
> >/proc/N/exe:  bash
> >/proc/N/comm: SCRIPT.sh
> > exec ./SCRIPT.sh:
> >/proc/N/exe:  bash
> >/proc/N/comm: SCRIPT.sh
> > sh ./SCRIPT.sh:
> >/proc/N/exe:  bash
> >/proc/N/comm: bash
> >
> > $ THIS_SH=/bin/ash ash comm.tests
> > ./SCRIPT.sh:
> >/proc/N/exe:  ash
> >/proc/N/comm: SCRIPT.sh
> > exec ./SCRIPT.sh:
> >/proc/N/exe:  ash
> >/proc/N/comm: SCRIPT.sh
> > sh ./SCRIPT.sh:
> >/proc/N/exe:  ash
> >/proc/N/comm: ash
> >
> > $ ash --help
> > BusyBox v1.35.0.git (2021-09-07 23:38:50 CEST) multi-call binary.
> > ...
> >
> > $ bash --help
> > GNU bash, version 5.0.17(1)-release-(x86_64-redhat-linux-gnu)
> > ...
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
--- a/libbb/appletlib.c
+++ b/libbb/appletlib.c
@@ -1065,8 +1065,13 @@ int main(int argc UNUSED_PARAM, char **a
 	 || ENABLE_FEATURE_PREFER_APPLETS
 	 || !BB_MMU
 	) {
-		if (NUM_APPLETS > 1)
-			set_task_comm(applet_name);
+		if (NUM_APPLETS > 1) {
+			char old_name[16];
+			get_task_comm(old_name);
+			if (strcmp(old_name, "exe") == 0) {
+set_task_comm(applet_name);
+			}
+		}
 	}
 
 	parse_config_file(); /* ...maybe, if FEATURE_SUID_CONFIG */
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1143,8 +1143,10 @@ void run_applet_no_and_exit(int a, const
 #endif
 #if defined(__linux__)
 void set_task_comm(const char *comm) FAST_FUNC;
+void get_task_comm(char *comm) FAST_FUNC;
 #else
 # define set_task_comm(name) ((void)0)
+# define get_task_comm(name) ((void)0)
 #endif
 
 /* Helpers for daemonization.
--- a/libbb/vfork_daemon_rexec.c
+++ b/libbb/vfork_daemon_rexec.c
@@ -33,6 +33,10 @@ void FAST_FUNC set_task_comm(const char
 	/* okay if too long (truncates) */
 	prctl(PR_SET_NAME, (long)comm, 0, 0, 0);
 }
+void FAST_FUNC get_task_comm(char *comm)
+{
+	prctl(PR_GET_NAME, (long)comm, 0, 0, 0);
+}
 #endif
 
 /*
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


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

2022-03-03 Thread Emmanuel Deloget
Hello Tim,

it seems to me that it should not be that complicated, as the tar
handling code is mostly encapsulated into a library of its own.

See https://git.busybox.net/busybox/tree/archival/libarchive for
futher reference.

Best regards,

-- Emmanuel Deloget


Le jeu. 3 mars 2022 à 18:59, Tim Tassonis  a écrit :
>
> Hi all
>
> I've written a package manager in C for my distro where packages are
> basically gzipped cpio (newc type) archives.
>
> I'm currently using the libarchive library in the program for
> extraction, which works well, but is quite a big dependency.
>
> So, as busybox handles those archives well (I tested it), I though it
> would be nice to get rid of libarchive and use the code from busybox. As
> my package manager is GPL, too, there should also be no licensing issues.
>
> However, I have not dug too deep into the busybox code yet and just
> wanted to ask whether someone knows if this is a good idea, e.g. how
> much work it would be to isolate the needed parts for use in my code.
>
> Has anybody else here ever done a similar thing, or is there maybe even
> a document giving a rough idea about what has to be considered or how to
> go about that the easiest?
>
> Sorry if this post is considered off-topic, I just though maybe someone
> has a quick answer that will save me some work.
>
>
> Bye
> Tim
>
>
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


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

2022-05-01 Thread Emmanuel Deloget
ntly store a seed file
then I shall provide another way to seed the RNG.

Fortunately, a lot of recent SoC have a hardware RNG which will
participate to /dev/random early on.

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

f() being a hash function with a salt + a (maybe not enough)
random value, I would think that new_key is going to be widely
different than old_key and more or less unpredictable, which is
what we want for a seed.

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

As you noted by yourself, it requires special code to read/write, so
unless this use-case is fully supported in busybox (with special
implementation for open/read/write), I don't see why it should have
an impact on the security of _all_ other devices.

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

I find at least one occurrence where the goal is to create a
random delay in udev-watch.c [1].

Anyway, and unless some weird security-oriented process decides
to read some random bytes without checking whether the random
pool in in a correct state (in which case I'm not sure you shall trust this
security-oriented process in the first place) the goal of seedrng is not
to protect all reads from /dev/urandom, but to make sure that we have
enough entropy to do the Real Work.

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

It seems to me that one might want to add a
CONFIG_FAVOR_SIZE_OVER_SECURITY configuration flag. I wish to say
that I'm not sure it's a good idea. Having a small busybox is a good thing, but
having a too small busybox that breaks products because of bad security
choices is not.

Best regards,

-- Emmanuel Deloget

[1] 
https://github.com/systemd/systemd/blob/d9338387d94ad611233cf0753801eefccfda432a/src/udev/udev-watch.c#L102
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


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

2022-05-01 Thread Emmanuel Deloget
Le lun. 2 mai 2022 à 03:31, Denys Vlasenko  a écrit :
>
> On Sun, May 1, 2022 at 6:35 PM Emmanuel Deloget  wrote:
> > > > - RNG is seeded and credited using file A.
> > > > - File A is unlinked but not fsync()d.
> > > > - TLS connection does something and a nonce is generated.
> > > > - System loses power and reboots.
> > > > - RNG is seeded and credited using same file A.
> > > > - TLS connection does something and the same nonce is generated,
> > > > resulting in catastrophic cryptographic failure.
> > > >
> > > > This is a big deal. Crediting seeds is not to be taken lightly.
> > > > Crediting file-based seeds is _only_ safe when they're never used
> > > > twice.
> > >
> > > Using the same file twice is better than having nothing at all.
> >
> > I beg to differ, and especially on some embedded systems where the RNG
> > might be quite controllable by an attacker from the outside (mostly because
> > it lacks a lot of entropy crediting inputs, which is exactly the reason why 
> > we
> > need seedrng in the first place). This may lead to catastrophic cryptography
> > failures down the road.
>
> Did you personally encounter such a situation?
> I'm not implying it's not really happening, I'm interested to hear
> from people who met this situation in real world.

Hey :) We're now entering in the "cautionary tales" territory :)

This happened, yes. We have not seen any exploitation of this in
nature (I would guess that it was mostly because there were other
lower hanging fruits; why would you attack the cryptography when you
could shellshock the device? yeah, bash was here -- it was a time
where ash was not deemed good enough to run the CGI scripts :) ; and
that was not the worst vulnerability so in practice you wouldn't even
need to use it to take full control of the device) but it was scary to
see that our different devices were able to boot and generate the same
SSH keys. This happened only on their first boot when the device was
not connected to the LAN (and not on all devices). Blank state, 0
source of entropy. Quite a "fun" time.

Best regards,

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


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

2022-05-02 Thread Emmanuel Deloget
Hi,

Le lun. 2 mai 2022 à 11:37, Denys Vlasenko  a écrit :
>
> On Mon, May 2, 2022 at 8:26 AM Emmanuel Deloget  wrote:
> > Le lun. 2 mai 2022 à 03:31, Denys Vlasenko  a 
> > écrit :
> > > > I beg to differ, and especially on some embedded systems where the RNG
> > > > might be quite controllable by an attacker from the outside (mostly 
> > > > because
> > > > it lacks a lot of entropy crediting inputs, which is exactly the reason 
> > > > why we
> > > > need seedrng in the first place). This may lead to catastrophic 
> > > > cryptography
> > > > failures down the road.
> > >
> > > Did you personally encounter such a situation?
> > > I'm not implying it's not really happening, I'm interested to hear
> > > from people who met this situation in real world.
> >
> > Hey :) We're now entering in the "cautionary tales" territory :)
> >
> > This happened, yes. We have not seen any exploitation of this in
> > nature (I would guess that it was mostly because there were other
> > lower hanging fruits; why would you attack the cryptography when you
> > could shellshock the device? yeah, bash was here -- it was a time
> > where ash was not deemed good enough to run the CGI scripts :) ; and
> > that was not the worst vulnerability so in practice you wouldn't even
> > need to use it to take full control of the device) but it was scary to
> > see that our different devices were able to boot and generate the same
> > SSH keys. This happened only on their first boot when the device was
> > not connected to the LAN (and not on all devices). Blank state, 0
> > source of entropy. Quite a "fun" time.
>
> If you still have a device which exhibits this behavior (seemingly
> totally deterministic state during each boot), can you try something like
>
> | xargs md5sum
>
> and see whether any /proc files display variability?

Unfortunately, I no longer have access to these devices. Not many were
sold, and I hope that none is still in activity today (although I
think that a former colleague told me that some of them are no later
than last year but I could be wrong). This was an old device
(development started in 2004, production stopped in 2009 IIRC).

Best regards,

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


Re: [PATCH] pw_encrypt: Add option to enable bcrypt support

2022-05-03 Thread Emmanuel Deloget
  $2b$ in your /etc/passwd or /etc/shadow files. These passwords
> +   are hashed using the bcrypt algorithm. Requires the use of a C
> +   library that supports bcrypt.
> +
> +config FEATURE_BCRYPT_COST
> +   int "bcrypt cost"
> +   range 4 31
> +   default 10
> +   depends on USE_BCRYPT
> +   help
> +   Cost paramter for the bcrypt hashing algorithm.

typo: parameter

> +   Specifies the number of rounds to use. Must be between 4 and 31,
> +   inclusive. This value is logarithmic, the actual number of
> +   iterations used will be 2**rounds – increasing the rounds by +1
> +   will double the amount of time taken.
> +
>   INSERT
>
>   endmenu
> diff --git a/loginutils/chpasswd.c b/loginutils/chpasswd.c
> index a032abbed..74673fa6f 100644
> --- a/loginutils/chpasswd.c
> +++ b/loginutils/chpasswd.c
> @@ -17,7 +17,8 @@
>   //config:  default "des"
>   //config:  depends on PASSWD || CRYPTPW || CHPASSWD
>   //config:  help
> -//config:  Possible choices are "d[es]", "m[d5]", "s[ha256]" or
> "sha512".
> +//config:  Possible choices are "d[es]", "m[d5]", "s[ha256]",
> +//config:  "sha512" or "b[crypt]" (when enabled).
>   //applet:IF_CHPASSWD(APPLET(chpasswd, BB_DIR_USR_SBIN, BB_SUID_DROP))
>
> --
> 2.34.1

Best regards,

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