On 21/06/2024 5:55 pm, Anthony PERARD wrote: > On Fri, Jun 21, 2024 at 05:16:56PM +0100, Andrew Cooper wrote: >> `xl devd` has been observed leaking /var/log/xldevd.log into children. >> >> Note this is specifically safe; dup2() leaves O_CLOEXEC disabled on newfd, so >> after setting up stdout/stderr, it's only the logfile fd which will close on >> exec(). >> >> Link: https://github.com/QubesOS/qubes-issues/issues/8292 >> Reported-by: Demi Marie Obenour <d...@invisiblethingslab.com> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> >> --- >> CC: Anthony PERARD <anth...@xenproject.org> >> CC: Juergen Gross <jgr...@suse.com> >> CC: Demi Marie Obenour <d...@invisiblethingslab.com> >> CC: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com> >> CC: Oleksii Kurochko <oleksii.kuroc...@gmail.com> >> >> Also entirely speculative based on the QubesOS ticket. >> >> v2: >> * Extend the commit message to explain why stdout/stderr aren't closed by >> this change >> >> For 4.19. This bugfix was posted earlier, but fell between the cracks. >> --- >> tools/xl/xl_utils.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/xl/xl_utils.c b/tools/xl/xl_utils.c >> index 17489d182954..060186db3a59 100644 >> --- a/tools/xl/xl_utils.c >> +++ b/tools/xl/xl_utils.c >> @@ -270,7 +270,7 @@ int do_daemonize(const char *name, const char *pidfile) >> exit(-1); >> } >> >> - CHK_SYSCALL(logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND, 0644)); >> + CHK_SYSCALL(logfile = open(fullname, O_WRONLY | O_CREAT | O_APPEND | >> O_CLOEXEC, 0644)); > Everytime we use O_CLOEXEC, we add in the C file > #ifndef O_CLOEXEC > #define O_CLOEXEC 0 > #endif > we don't need to do that anymore? > Or I guess we'll see if someone complain when they try to build on an > ancien version of Linux. > > Acked-by: Anthony PERARD <anthony.per...@vates.tech>
Thanks. I did originally have that ifdefary here, but then I noticed that this isn't the first instance like this in xl, and I'm going to be using that as a justification soon to explicitly drop support for Linux < 2.6.23. ~Andrew
