On Tue, Nov 7, 2023 at 7:27 PM Louai Al-Khanji <lo...@astranis.com> wrote:
> On Tue, Nov 7, 2023 at 8:03 AM Denys Vlasenko <vda.li...@googlemail.com> 
> wrote:
> > On Fri, Nov 3, 2023 at 11:31 PM Louai Al-Khanji <lo...@astranis.com> wrote:
> > > Attached is a proposed patch. Any feedback would be appreciated.
> >
> > My experiments with ssd version 1.21.22 show that the file is opened with
> > O_CREAT|O_APPEND, and it does not allow -O without -b.
> >
> > If execv fails, error message goes to this file.
> > IOW: there is no need to save/restore old stderr fd. Just replace it
> > with the new fd
> > (and don't forget to not leak any extra open fds).
>
> Thank you for the feedback everyone. New version attached.
>
> It looked a little tricky to me to add the logic around
> bb_daemon_helper() since it closes open fds. Maybe I am missing
> something.
>
> The code now checks the args more strictly and prints usage if -O is
> given without -b.
>
> I dropped restoring of the stdout/stderr fds. I believe this patch
> cannot leak fds.
>
> One question I have is whether it's okay to lose error messages. On
> failure to open the output file I believe the error message currently
> goes into the void. Same if any of the dup2 calls or the close call
> fails.

Yes, that's not good.
I tried to fix it in git now. Please try.


> BTW I noticed that bb_daemon_helper() internally calls setsid()
> already, so the extra call in start_stop_daemon.c seems superfluous. I
> didn't however touch that in this patch.

No:

#define bb_daemon_helper(arg) bb_daemonize_or_rexec((arg) |
DAEMON_ONLY_SANITIZE, NULL)

DAEMON_ONLY_SANITIZE bit prevents setsid() and daemonization (the vfork).
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to