Re: [PATCH] syslogd: fork after init on a regular system, not before

2023-10-05 Thread Michael Tokarev

03.10.2023 20:16, Denys Vlasenko:

Pushed to git, thank you.


Thank you for that.  There's one more issue in there with remote logging,
see this message for the patch:
http://lists.busybox.net/pipermail/busybox/2023-September/090499.html

/mjt


On Sun, Jun 18, 2023 at 9:24 AM Michael Tokarev  wrote:


14.06.2023 23:58, Michael Tokarev wrote:

14.06.2023 23:52, Michael Tokarev wrote:

Actually it doesn't work on regular system too, the fd#0 is redirected
from /dev/null somewhere down the line and enters a tight loop


Attached is the fix. Not touching daemonize_or_reexec().

Signed-off-by: Michael Tokarev 


Ping?
syslogd is completely broken in master now.



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


Re: [PATCH] syslogd: fork after init on a regular system, not before

2023-10-03 Thread Denys Vlasenko
Pushed to git, thank you.

On Sun, Jun 18, 2023 at 9:24 AM Michael Tokarev  wrote:
>
> 14.06.2023 23:58, Michael Tokarev wrote:
> > 14.06.2023 23:52, Michael Tokarev wrote:
> >> Actually it doesn't work on regular system too, the fd#0 is redirected
> >> from /dev/null somewhere down the line and enters a tight loop
> >
> > Attached is the fix. Not touching daemonize_or_reexec().
> >
> > Signed-off-by: Michael Tokarev 
>
> Ping?
> syslogd is completely broken in master now.
>
> Thanks,
>
> /mjt
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] syslogd: fork after init on a regular system, not before

2023-06-18 Thread Michael Tokarev

14.06.2023 23:58, Michael Tokarev wrote:

14.06.2023 23:52, Michael Tokarev wrote:

Actually it doesn't work on regular system too, the fd#0 is redirected
from /dev/null somewhere down the line and enters a tight loop


Attached is the fix. Not touching daemonize_or_reexec().

Signed-off-by: Michael Tokarev 


Ping?
syslogd is completely broken in master now.

Thanks,

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


Re: [PATCH] syslogd: fork after init on a regular system, not before

2023-06-14 Thread Michael Tokarev

14.06.2023 23:52, Michael Tokarev wrote:

Actually it doesn't work on regular system too, the fd#0 is redirected
from /dev/null somewhere down the line and enters a tight loop


Attached is the fix. Not touching daemonize_or_reexec().

Signed-off-by: Michael Tokarev 

--- busybox.orig/sysklogd/syslogd.c
+++ busybox/sysklogd/syslogd.c
@@ -1009,6 +1009,7 @@ static int try_to_resolve_remote(remoteH
 static int NOINLINE syslogd_init(char **argv)
 {
 	int opts;
+	int fd;
 	char OPTION_DECL;
 #if ENABLE_FEATURE_REMOTE_LOG
 	llist_t *remoteAddrList = NULL;
@@ -1056,7 +1057,7 @@ static int NOINLINE syslogd_init(char **
 	G.hostname = safe_gethostname();
 	*strchrnul(G.hostname, '.') = '\0';
 
-	xmove_fd(create_socket(), STDIN_FILENO);
+	fd = create_socket();
 
 	if (opts & OPT_circularlog)
 		ipcsyslog_init();
@@ -1068,6 +1069,8 @@ static int NOINLINE syslogd_init(char **
 		bb_daemonize_or_rexec(DAEMON_CHDIR_ROOT, argv);
 	}
 
+	xmove_fd(fd, STDIN_FILENO);
+
 	/* Set up signal handlers (so that they interrupt read()) */
 	signal_no_SA_RESTART_empty_mask(SIGTERM, record_signo);
 	signal_no_SA_RESTART_empty_mask(SIGINT, record_signo);
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] syslogd: fork after init on a regular system, not before

2023-06-14 Thread Michael Tokarev

14.06.2023 23:30, Michael Tokarev пишет:

13.06.2023 17:30, Denys Vlasenko wrote:

I think we can delay daemonizing in both cases.

Please try current git.


Yes, your version is better, it is definitely cleaner, especially
after the preparational patch.

But does it work for non-MMU case?
Or will it just open /dev/log twice and everything will be ok?


Actually it doesn't work on regular system too, the fd#0 is redirected
from /dev/null somewhere down the line and enters a tight loop:

44138 readlink("/dev/log", "/run/systemd/journal/dev-log", 80) = 28
44138 readlink("/run/systemd/journal/dev-log", 0x5611c6c26ee0, 80) = -1 EINVAL 
(Invalid argument)
44138 unlink("/run/systemd/journal/dev-log") = 0
44138 socket(AF_UNIX, SOCK_DGRAM, 0)= 3
44138 bind(3, {sa_family=AF_UNIX, sun_path="/run/systemd/journal/dev-log"}, 
110) = 0
44138 chmod("/dev/log", 0666)   = 0
44138 dup2(3, 0)= 0
44138 close(3)  = 0
44138 chdir("/")= 0
44138 openat(AT_FDCWD, "/dev/null", O_RDWR) = 3
44138 clone(child_stack=NULL, 
flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, 
child_tidptr=0x7fb6bc72fa10) = 44139
44138 exit_group(0) = ?
44138 +++ exited with 0 +++
44139 set_robust_list(0x7fb6bc72fa20, 24) = 0
44139 setsid()  = 44139
44139 dup2(3, 0)= 0
44139 dup2(3, 1)= 1
44139 dup2(3, 2)= 2
44139 close(3)  = 0

It is done in bb_daemonize_or_rexec(). Which, it seems,
can be optimized too, to avoid duplicate dup(0,1,2) case.

This is why I used extra variable 'int fd = .. xmove_fd(fd)'.

(btw, should it chmod the result of readlink, not /dev/log? - probably doesn't 
matter).

Thanks,

/mjt

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


Re: [PATCH] syslogd: fork after init on a regular system, not before

2023-06-14 Thread Michael Tokarev

13.06.2023 17:30, Denys Vlasenko wrote:

I think we can delay daemonizing in both cases.

Please try current git.


Yes, your version is better, it is definitely cleaner, especially
after the preparational patch.

But does it work for non-MMU case?
Or will it just open /dev/log twice and everything will be ok?

I'm not sure how important the non-MMU case is these days, to begin with.

The prob there is that you open stuff in parent and check for errors,
that's okay. But now you repeat the same open again in a new process
after exec, and you can only _hope_ things will work the same way one
more time, because you just did that a moment before.  This is still
a bit dirty at least, I think. But "generally" will work.

Either way, this version definitely works ok on a regular system with MMU.

The same problem exists in klogd as well.

Thank you!

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


Re: [PATCH] syslogd: fork after init on a regular system, not before

2023-06-13 Thread Denys Vlasenko
I think we can delay daemonizing in both cases.

Please try current git.

On Thu, Jun 8, 2023 at 10:44 AM Michael Tokarev  wrote:
>
> Current syslogd performs all init after daemonizing, meanwhile
> main process exits successfully. This means any errors during init
> will not be even shown up because at this time the process has its
> stderr redirected to /dev/null already.
>
> On a MMU system, delay daemonizing to after init.
> On non-MMU system, keep current code.
>
> Signed-off-by: Michael Tokarev 
> ---
> This is a generic problem with many busybox daemons, I think its
> root is within the no-MMU system support (where we can't fork).
> I think this should be solved in a more generic way too, but this
> is just an example of how this can be fixed in principle.  The
> prob with syslogd is real, any error and it doesn't start but
> does not show error messages and reports success, - this is hardly
> acceptable behavour.
>
>  sysklogd/syslogd.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/sysklogd/syslogd.c b/sysklogd/syslogd.c
> index 6ddfd771a..2f9a727cd 100644
> --- a/sysklogd/syslogd.c
> +++ b/sysklogd/syslogd.c
> @@ -1025,7 +1025,6 @@ static void do_syslogd(void)
> signal(SIGALRM, do_mark);
> alarm(G.markInterval);
>  #endif
> -   xmove_fd(create_socket(), STDIN_FILENO);
>
> if (option_mask32 & OPT_circularlog)
> ipcsyslog_init();
> @@ -1033,6 +1032,16 @@ static void do_syslogd(void)
> if (option_mask32 & OPT_kmsg)
> kmsg_init();
>
> +   {
> +   int fd = create_socket();
> +#if BB_MMU
> +   if (!(option_mask32 & OPT_nofork)) {
> +   bb_daemonize(DAEMON_CHDIR_ROOT);
> +   }
> +#endif
> +   xmove_fd(fd, STDIN_FILENO);
> +   }
> +
> timestamp_and_log_internal("syslogd started: BusyBox v" BB_VER);
> write_pidfile_std_path_and_ext("syslogd");
>
> @@ -1179,9 +1188,11 @@ int syslogd_main(int argc UNUSED_PARAM, char **argv)
> G.hostname = safe_gethostname();
> *strchrnul(G.hostname, '.') = '\0';
>
> +#if !BB_MMU
> if (!(opts & OPT_nofork)) {
> bb_daemonize_or_rexec(DAEMON_CHDIR_ROOT, argv);
> }
> +#endif
>
> do_syslogd();
> /* return EXIT_SUCCESS; */
> ___
> 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