bb_daemonize_or_rexec(DAEMON_CLOSE_EXTRA_FDS) does not close extra fds

[Bernd Petrovitsch]

Hi all!

On 14/03/2019 16:35, Arnout Vandecappelle wrote:
[...]
>  I've noticed that bb_daemonize_or_rexec(DAEMON_CLOSE_EXTRA_FDS +
> DAEMON_DEVNULL_STDIO) does not actually close all fds. This is what gets called
> by start-stop-daemon -b, so it means that start-stop-daemon will propagate all
> non-CLOEXEC fds from the parent.
> 
>  To test:
> 
> sh -c 'exec 3>&1; busybox start-stop-daemon -S -b -x /bin/sleep -- 10000'
> ls -l /proc/$(pidof sleep)/fd
> 
> 
>  This happens because we do:
> 
> 	if (flags & DAEMON_DEVNULL_STDIO) {
> 		close(0);
> 		close(1);
> 		close(2);
> 	}
> 
> 	fd = open(bb_dev_null, O_RDWR);
> ...
> 	while (fd > 2) {
> 		close(fd--);
> 	}
> 
>  Since we just did close(0), fd will be 0.

Moving that further up works better but doesn't really help.
And IMHO fds 0-2 shouldn't be closed when the program ie exec'ed
as practically all Unix software assumes that they are open inherited.

>  The problem is that the assumption that open() returns an fd that is larger
> than all existing fds is simply wrong.

Yup, the lowest closed is taken.

>  I think that there is no other way to close all open fds than to iterate over
> /proc/self/fd. But that is of course not very portable.

And slow and (probably) fat ...

>  Thoughts?

https://stackoverflow.com/questions/1315382/closing-all-open-files-in-a-process
has something:
- Use https://man.openbsd.org/closefrom.2 - which doesn't exist on
  Linux.
- use getdtablesize()
- use sysconf(_SC_OPEN_MAX)

MfG,
	Bernd
-- 
"I dislike type abstraction if it has no real reason. And saving
on typing is not a good reason - if your typing speed is the main
issue when you're coding, you're doing something seriously wrong."
    - Linus Torvalds
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pEpkey.asc
Type: application/pgp-keys
Size: 2472 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20190314/def3d001/attachment.key>