[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 12/26] qga: replace pipe() with g_unix_open_pipe(CLOEXEC)
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v2 12/26] qga: replace pipe() with g_unix_open_pipe(CLOEXEC) |
Date: |
Tue, 3 May 2022 12:22:55 +0100 |
User-agent: |
Mutt/2.1.5 (2021-12-30) |
On Wed, Apr 27, 2022 at 12:24:44PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Wed, Apr 27, 2022 at 5:08 AM Richard Henderson <
> richard.henderson@linaro.org> wrote:
>
> > On 4/26/22 02:27, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > qga/commands-posix.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > > index 77f4672ca2c9..094487c2c395 100644
> > > --- a/qga/commands-posix.c
> > > +++ b/qga/commands-posix.c
> > > @@ -2529,7 +2529,7 @@ void qmp_guest_set_user_password(const char
> > *username,
> > > goto out;
> > > }
> > >
> > > - if (pipe(datafd) < 0) {
> > > + if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
> > > error_setg(errp, "cannot create pipe FDs");
> > > goto out;
> > > }
> >
> > This looks wrong, since the next thing that happens is fork+execl.
> >
> >
> Before exec(), it does
> close(datafd[1]);
> dup2(datafd[0], 0);
>
> 0, the newfd, does not share file descriptor flags (the close-on-exec flag).
>
> I did a quick test, and it seems to be fine.
The 'dup' man page says
The two file descriptors do not share file descriptor flags
(the close-on-exec flag). The close-on-exec flag (FD_CLOEXEC;
see fcntl(2)) for the duplicate descriptor is off.
so we're fine in this respect. You could need to use dup3 to explicitly
turn on FD_CLOEXEC on the duplicate, so
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v2 12/26] qga: replace pipe() with g_unix_open_pipe(CLOEXEC),
Daniel P . Berrangé <=