Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

[Takashi Yano <takashi.yano@nifty.ne.jp>]

I am sorry for being absent for a long time.

On Sun, 26 Dec 2021 10:09:57 -0500
Ken Brown wrote:
> On 12/25/2021 11:56 PM, Jeremy Drake wrote:
> > I set up a windows server 2022 VM last night and went nuts stressing
> > pacman/GPGME.  I was able to reproduce the issue there:
> > 
> > status = 0x00000000, phi->NumberOfHandles = 8261392, n_handle = 256
> > [#####----------------------------------]  14%
> > assertion "phi->NumberOfHandles <= n_handle" failed: file
> > "../../.././winsup/cygwin/fhandler_pipe.cc", line 1281, function: void*
> > fhandler_pipe::get_query_hdl_per_process(WCHAR*, OBJECT_NAME_INFORMATION*)
> > 
> > So it is not something inherent in the x86_64-on-ARM64 emulation but can
> > happen on native x86_64 also.
> 
> A Google search led me to something that might explain what's going on.  Look at 
> the function PhEnumHandlesEx2 starting at line 5713 in
> 
>   https://github.com/processhacker/processhacker/blob/master/phlib/native.c#L5152

Thank you very much for finding this,

> Two interesting things:
> 
> 1. For some processes, NtQueryInformationProcess(ProcessHandleInformation) can 
> return STATUS_SUCCESS with invalid handle information.  See the comment starting 
> at line 5754, where it is shown how to detect this.

and also applying the fix.

> 2. You can use the ReturnLength parameter of NtQueryInformationProcess to see 
> how big a buffer is needed.  This might be more efficient than repeatedly 
> doubling the buffer size.

Even if ReturnLength is used, the first NtQueryInformationProcess()
call and the second NtQueryInformationProcess() call will not be
done in atomic, so retrying is still necessary. However, it may be
more efficient as you mentioned.

The similar is true also for NtQuerySystemInformation().

Do you still think it is better to use ReturnLength rather than
doubling the buffer?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>