[PATCH 1/2] ash: Set the CLOEXEC bit in fcntl_F_DUPFD

[Denys Vlasenko]

Thanks!

I applied your patch with some minor changes.

On Fri, Mar 30, 2018 at 1:40 PM, Mark Marshall <markmarshall14 at gmail.com> wrote:
> Hi.
>
> I have a small reproducer.  Place the following script on the target device:
>
> z1.sh
> =====
> sleep 20 </dev/null >/dev/null 2>/dev/null &
> PID=$!
> echo "self"
> ls -lv /proc/self/fd
> echo "child"
> ls -lv /proc/${PID}/fd
>
> Then, run this script via ssh/sshd from another machine, with 1.24.1
> the output is as follows:
>
> marmar01 at marmar13:~/develop/oeltron$ ssh root at 192.168.0.252 "/tmp/z1.sh"
> self
> lr-x------    1 root     root            64 Mar 30 10:14 0 -> pipe:[7874]
> l-wx------    1 root     root            64 Mar 30 10:14 1 -> pipe:[7875]
> l-wx------    1 root     root            64 Mar 30 10:14 2 -> pipe:[7876]
> lr-x------    1 root     root            64 Mar 30 10:14 3
> ls: /proc/self/fd/3: cannot read link: No such file or directory
> child
> lr-x------    1 root     root            64 Mar 30 10:14 0 -> /dev/null
> l-wx------    1 root     root            64 Mar 30 10:14 1 -> /dev/null
> l-wx------    1 root     root            64 Mar 30 10:14 2 -> /dev/null
>
>
> With 1.28.1 the output is this:
>
> marmar01 at marmar13:~/develop/oeltron$ ssh root at 192.168.0.252 "/tmp/z1.sh"
> self
> lr-x------    1 root     root            64 Mar 30 10:16 0 -> pipe:[7736]
> l-wx------    1 root     root            64 Mar 30 10:16 1 -> pipe:[7737]
> l-wx------    1 root     root            64 Mar 30 10:16 2 -> pipe:[7738]
> lr-x------    1 root     root            64 Mar 30 10:16 3
> ls: /proc/self/fd/3: cannot read link: No such file or directory
> child
> lr-x------    1 root     root            64 Mar 30 10:16 0 -> /dev/null
> l-wx------    1 root     root            64 Mar 30 10:16 1 -> /dev/null
> l-wx------    1 root     root            64 Mar 30 10:16 2 -> /dev/null
> lr-x------    1 root     root            64 Mar 30 10:16 10 -> /dev/null
> l-wx------    1 root     root            64 Mar 30 10:16 11 -> pipe:[7737]
> l-wx------    1 root     root            64 Mar 30 10:16 12 -> pipe:[7738]
>
> Additionally (and more importantly) with the broken 1.28.1 case the
> ssh session waits until the sleep
> finishes (20 seconds).  In the real case that I care about the program
> being run is not a sleep, but is
> a daemon that never exits.
>
> My patch, on top of 1.28.1, makes things work like 1.24.1 again.
>
> Regards,
> Mark
>
>
> On 28 March 2018 at 17:06, Denys Vlasenko <vda.linux at googlemail.com> wrote:
>> On Mon, Mar 19, 2018 at 10:31 AM,  <mark.marshall at omicronenergy.com> wrote:
>>> From: Mark Marshall <mark.marshall at omicronenergy.com>
>>>
>>> The function fcntl_F_DUPFD was introduced as some re-factoring at commit
>>> 035486c7500c09616a6c1040d8e70923532a5c2d
>>> "ash: significant overhaul of redirect saving logic".
>>> The old code used to set the CLOEXEC bit, but this got lost in the
>>> re-factoring, so this change makes the new code do the same.
>>>
>>> This was found because if a script was run via ssh, and this script ran
>>> a second script, redirecting its stdin and stdout, and putting it into
>>> the background, the ssh would hang waiting for the backgrounded script
>>> to terminate.  From /proc/999/fd it can be seen that the background script
>>> still has the stdin pipe from sshd open (at file handle 10+).
>>
>> Please provide a reproducer. I tried this:
>>
>> z1
>> ==
>> ./busybox ash z2 </dev/null >/dev/null 2>/dev/null &
>> echo Ok
>>
>> z2
>> ==
>> ls -l /proc/$$/fd   >z2.out1 2>&1
>> ls -l /proc/self/fd >z2.out2 2>&1
>>
>> I do see some extra fds.
>>
>> The thing is, when I try it, there is no difference before
>> and after commit 035486c7500c09616a6c1040d8e70923532a5c2d.
>> _______________________________________________
>> busybox mailing list
>> busybox at busybox.net
>> http://lists.busybox.net/mailman/listinfo/busybox