Re: [PATCH] tools/libxs: Open /dev/xen/xenbus fds as O_CLOEXEC

[Jürgen Groß <jgross@xxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
With the style breakage below fixed:

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>

> ---
> CC: Anthony PERARD <anthony@xxxxxxxxxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> CC: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
> CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
>
> 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