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)); > free(fullname); > assert(logfile >= 3);
Definitely an improvement. I would also add O_NOCTTY to work around a particularly unfortunate Linux kernel design decision, but that can either be fixed up on commit or be a separate patch. Reviewed-by: Demi Marie Obenour <d...@invisiblethingslab.com> -- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab
signature.asc
Description: PGP signature
