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

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

[Takashi Yano]

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>

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

[Ken Brown]

On 1/13/2022 5:56 AM, Takashi Yano wrote:
> Ken Brown wrote:
>> 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?

I'm not sure.  I only mentioned it because I saw that that's what Process Hacker 
did, but still in a retry loop as you said.

I suspect it doesn't make a lot of difference in practice, since we call the 
function once and then cache the value.  Do whatever you think is best.

Thanks.

Ken