> -----Original Message----- > From: Yan Vugenfirer [mailto:yvuge...@redhat.com] > Sent: Wednesday, May 21, 2014 5:01 PM > To: Michael Roth > Cc: Luiz Capitulino; Wangrui (K); Gonglei (Arei); qemu-devel@nongnu.org; > arm...@redhat.com > Subject: Re: [Qemu-devel] [PATCH] qga: Fix handle fd leak in > acquire_privilege() > > > On May 20, 2014, at 10:46 PM, Michael Roth <mdr...@linux.vnet.ibm.com> > wrote: > > > Quoting Luiz Capitulino (2014-05-20 14:17:42) > >> On Mon, 19 May 2014 15:26:03 +0800 > >> <arei.gong...@huawei.com> wrote: > >> > >>> From: Gonglei <arei.gong...@huawei.com> > >>> > >>> token should be closed in all conditions. > >>> So move CloseHandle(token) to "out" branch. > >> > >> Looks good to me. Michael, are you going to pick this one? > > > > Sure I'll queue it. Though i'm a bit surprised OpenProcessToken() will still > > return an open handle on failure. Is there any confirmation a handle is > > actually getting leaked here, as opposed to just returning a handle that's > > already been closed? > > It won't return a handle if it failed (I actually was going to reject the > patch > because of it) - but if you look closely at the code you will see error cases > after > OpenProcessToken was successful where we jump out of the if scope and then > CloseHandle won't be called. >
Yep. The two "goto out"s are in the condition that OpenProcessToken returns successfully. > Best regards, > Yan. > > > > > If it's the latter case we might end up closing it twice after the change, > > so just want to confirm. > > > >> > >>> > >>> Signed-off-by: Wang Rui <moon.wang...@huawei.com> > >>> Signed-off-by: Gonglei <arei.gong...@huawei.com> > >>> --- > >>> qga/commands-win32.c | 6 ++++-- > >>> 1 file changed, 4 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c > >>> index d793dd0..e769396 100644 > >>> --- a/qga/commands-win32.c > >>> +++ b/qga/commands-win32.c > >>> @@ -31,7 +31,7 @@ > >>> > >>> static void acquire_privilege(const char *name, Error **errp) > >>> { > >>> - HANDLE token; > >>> + HANDLE token = NULL; > >>> TOKEN_PRIVILEGES priv; > >>> Error *local_err = NULL; > >>> > >>> @@ -53,13 +53,15 @@ static void acquire_privilege(const char *name, > Error **errp) > >>> goto out; > >>> } > >>> > >>> - CloseHandle(token); > >>> } else { > >>> error_set(&local_err, QERR_QGA_COMMAND_FAILED, > >>> "failed to open privilege token"); > >>> } > >>> > >>> out: > >>> + if (token) { > >>> + CloseHandle(token); > >>> + } > >>> if (local_err) { > >>> error_propagate(errp, local_err); > >>> } > > > >