| Message ID | 20240504011614.1645851-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/libxs: Open /dev/xen/xenbus fds as O_CLOEXEC | expand |
On 04.05.24 03:16, Andrew Cooper wrote: > The header description for xs_open() goes as far as to suggest that the fd is > O_CLOEXEC, but it isn't actually. > > `xl devd` has been observed leaking /dev/xen/xenbus into children. > > Link: https://github.com/QubesOS/qubes-issues/issues/8292 > Reported-by: Demi Marie Obenour <demi@invisiblethingslab.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> With the style breakage below fixed: Reviewed-by: Juergen Gross <jgross@suse.com> > --- > CC: Anthony PERARD <anthony@xenproject.org> > CC: Juergen Gross <jgross@suse.com> > CC: Demi Marie Obenour <demi@invisiblethingslab.com> > CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > Entirely speculative patch based on a Matrix report > --- > tools/libs/store/xs.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c > index 140b9a28395e..1f74fb3c44a2 100644 > --- a/tools/libs/store/xs.c > +++ b/tools/libs/store/xs.c > @@ -54,6 +54,10 @@ struct xs_stored_msg { > #include <dlfcn.h> > #endif > > +#ifndef O_CLOEXEC > +#define O_CLOEXEC 0 > +#endif > + > struct xs_handle { > /* Communications channel to xenstore daemon. */ > int fd; > @@ -227,7 +231,7 @@ static int get_socket(const char *connect_to) > static int get_dev(const char *connect_to) > { > /* We cannot open read-only because requests are writes */ > - return open(connect_to, O_RDWR); > + return open(connect_to, O_RDWR|O_CLOEXEC); Nit: spaces around the "|", please. Juergen > } > > static int all_restrict_cb(Xentoolcore__Active_Handle *ah, domid_t domid) { > > base-commit: feb9158a620040846d76981acbe8ea9e2255a07b
diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c index 140b9a28395e..1f74fb3c44a2 100644 --- a/tools/libs/store/xs.c +++ b/tools/libs/store/xs.c @@ -54,6 +54,10 @@ struct xs_stored_msg { #include <dlfcn.h> #endif +#ifndef O_CLOEXEC +#define O_CLOEXEC 0 +#endif + struct xs_handle { /* Communications channel to xenstore daemon. */ int fd; @@ -227,7 +231,7 @@ static int get_socket(const char *connect_to) static int get_dev(const char *connect_to) { /* We cannot open read-only because requests are writes */ - return open(connect_to, O_RDWR); + return open(connect_to, O_RDWR|O_CLOEXEC); } static int all_restrict_cb(Xentoolcore__Active_Handle *ah, domid_t domid) {
The header description for xs_open() goes as far as to suggest that the fd is O_CLOEXEC, but it isn't actually. `xl devd` has been observed leaking /dev/xen/xenbus into children. Link: https://github.com/QubesOS/qubes-issues/issues/8292 Reported-by: Demi Marie Obenour <demi@invisiblethingslab.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Anthony PERARD <anthony@xenproject.org> CC: Juergen Gross <jgross@suse.com> CC: Demi Marie Obenour <demi@invisiblethingslab.com> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Entirely speculative patch based on a Matrix report --- tools/libs/store/xs.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) base-commit: feb9158a620040846d76981acbe8ea9e2255a07b