[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: interesting segfault when running awk as pid==1
From:       Michael Tokarev <mjt () tls ! msk ! ru>
Date:       2012-07-07 20:55:00
Message-ID: 4FF8A224.6010708 () msgid ! tls ! msk ! ru
[Download RAW message or body]

On 07.07.2012 23:33, Denys Vlasenko wrote:
[]
>>> The debian-specific patch in question is this (one-liner):
>>>
>>> shell/ash.c:
>>> -       { VSTRFIXED|VTEXTFIXED       , bb_PATH_root_path, changepath },
>>> +       { VSTRFIXED|VTEXTFIXED|VEXPORT, bb_PATH_root_path, changepath },
>>>
>>> so it merely exports PATH variable, and should not affect
>>> the behavour in question, but I'm not sure anymore about
>>> this.  It looks rather innocent.
>>
>> Apparently, it makes it so that ash exports $PATH.
>> If $PATH was in the set of inherited variables,
>> then it is reused, and this apparently works.
>> But in init, $PATH may be _not_ inherited.
>> So ash sets its own $PATH (I just tested it):
>>
>> PATH=/sbin:/usr/sbin:/bin:/usr/bin
>>
>> Basically, these two scenarios are not exactly the same.
>>
>> Try reproducing segfault by running ahs with empty environment:
>>
>> env - /path/to/busybox ash
>>
>> Does it segfault if you run awk from it?
> 
> Reproduced.
> 
>         if (environ) for (envp = environ; *envp; envp++) {
>                 /* environ is writable, thus we don't strdup it needlessly */
>                 char *s = *envp;
>                 char *s1 = strchr(s, '=');
>                 if (s1) {
>                         *s1 = '\0';
> 
> It happens in the above line.
> 
> Caused by: ash.c:
> 
> static void
> tryexec(IF_FEATURE_SH_STANDALONE(int applet_no,) char *cmd, char
> **argv, char **envp)
> {
> #if ENABLE_FEATURE_SH_STANDALONE
>         if (applet_no >= 0) {
>                 if (APPLET_IS_NOEXEC(applet_no)) {
>                         clearenv();
>                         while (*envp)
>                                 putenv(*envp++);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                         run_applet_no_and_exit(applet_no, argv);
>                 }
>                 /* re-exec ourselves with the new arguments */
>                 execve(bb_busybox_exec_path, argv, envp);
>                 /* If they called chroot or otherwise made the binary no longer
>                  * executable, fall through */
>         }
> #endif
> 
> Basically, we putenv() a string which is constant,
> and when we try to modify it, we segfault.

Thank you Denys, thank you very much for figuring it out.

I was about to add printfs here and there, planned to do
that today evening.

> I think Debian's patch does something which is not supported.

Funny you mention it.  The thing is: with the patch,
there's just ONE variable marked as VEXPORT, and no
variable is marked as such without the patch.

So I'm not surprized it doesn't work.

Looking at the other places, everywhere where VEXPORT
flag is used, the variable in question is dynamically
allocated.  So setting this flag for initial value is
indeed not supported -- maybe it is a good idea to
document this fact in the code.

Now, for the root issue -- exporting $PATH from [a]sh --
I inherited this patch together with busybox debian
package, and am now trying to understand where it
comes from and for what.  I'll drop it from the next
Debian release (wheezy+1), since neither bash nor
dash exports $PATH by default.  But for now, I'll
have to change the way stdpath is set up, to allow
VEXPORT to work correctly.

Thank you very much for finding the root of the issue!

/mjt
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]