[PATCH] crond: add LOG_NDELAY to openlog

Jones Syue 薛懷宗 jonessyue at qnap.com
Thu Jan 4 10:20:16 UTC 2024


> I'd actually suggest using fork() - much safer.
> While slightly slower no one is going to notice the difference
> when cron is running programs.

Yes. No doubt about it! :)
Replace vfork() with fork()[1] could way around this issue[2], since fork() is 
copy-on-write so each children process has its own address space and its 
own copy of 'static var LogFile' in data segment, it is not shared any more
among all processes including parent and children, and any modification 
from children is not visible to parent.

Per my environment we did have some entry-level embedded systems with much 
less memory and very limited computation power, so vfork() implementation 
is preferred than fork() since vfork() is more effective on memory footprint
and process creation speed. Really a BIG FAN of how busybox explains vfork():
https://busybox.net/FAQ.html#tips_vfork

So it looks like simply adds LOG_NDELAY might be a feasible approach to 
address this issue :)

[1] Replace vfork() with fork()

--- a/miscutils/crond.c
+++ b/miscutils/crond.c
@@ -713,7 +713,7 @@ fork_job(const char *user, int mailFd, CronLine *line, bool run_sendmail)
        set_env_vars(pas, shell, NULL); /* don't use crontab's PATH for sendmail */

        sv_logmode = logmode;
-       pid = vfork();
+       pid = fork();
        if (pid == 0) {
                /* CHILD */
                /* initgroups, setgid, setuid, and chdir to home or CRON_DIR */

[2] Use strace to understand how fork() looks like.
The crond parent cloned two children pid is 25660 and 25661.
parent has its own consistent 'LogFile = 5' and DGRAM socket fd = 5.

# grep -rE "^clone|^vfork|^sock.*UNIX.*DGRAM|connect.*log|sendto.*75|can\'|parse error|change directory" 20231213_01.log.20290
clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f65ac34a9d0) = 25660
clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f65ac34a9d0) = 25661
socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0) = 5
connect(5, {sa_family=AF_UNIX, sun_path="/dev/log"}, 110) = 0
sendto(5, "<75>Dec 13 14:11:00 crond[20290]: user admin: parse error at <\n", 63, MSG_NOSIGNAL, NULL, 0) = 63

The children pid=25660 and 25661 both has its own consistent 
'LogFile = 4' and DGRAM socket fd = 4.

# grep -rE "^vfork|^sock.*UNIX.*DGRAM|connect.*log|sendto.*75|can\'|parse error|change directory" 20231213_01.log.25660
socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0) = 4
connect(4, {sa_family=AF_UNIX, sun_path="/dev/log"}, 110) = 0
sendto(4, "<75>Dec 13 14:10:02 crond[25660]: can't change directory to '/share/homes/admin'\n", 81, MSG_NOSIGNAL, NULL, 0) = 81

# grep -rE "^vfork|^sock.*UNIX.*DGRAM|connect.*log|sendto.*75|can\'|parse error|change directory" 20231213_01.log.25661
socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0) = 4
connect(4, {sa_family=AF_UNIX, sun_path="/dev/log"}, 110) = 0
sendto(4, "<75>Dec 13 14:10:02 crond[25661]: can't change directory to '/share/homes/admin'\n", 81, MSG_NOSIGNAL, NULL, 0) = 81

--

Regards,
Jones Syue | 薛懷宗
QNAP Systems, Inc.


More information about the busybox mailing list