[PATCH] fhandler_pipe: add sanity limit to handle loops
Jeremy Drake
cygwin@jdrake.com
Sun Dec 26 22:43:58 GMT 2021
On Sun, 26 Dec 2021, Ken Brown wrote:
> + /* NtQueryInformationProcess can return STATUS_SUCCESS with
> + invalid handle data for certain processes. See
> + https://github.com/processhacker/processhacker/blob/master/phlib/native.c#L5754.
I would recommend using the "permalink" to the line, since future
commits could change both the line number and even the comment you are
referring to.
https://github.com/processhacker/processhacker/blob/05f5e9fa477dcaa1709d9518170d18e1b3b8330d/phlib/native.c#L5754
> + We need to ensure that NumberOfHandles is zero in this
> + case to avoid a crash in the loop below. */
> + phi->NumberOfHandles = 0;
> status = NtQueryInformationProcess (proc, ProcessHandleInformation,
> phi, nbytes, &len);
> if (NT_SUCCESS (status))
Would it make sense to leave an assert (phi->NumberOfHandles <= n_handle)
before the for loop too just in case something odd happens in the
future? That made it a lot easier to know what was going on.
My loops are still going after an hour. I know that ARM64 would have hit
the assert before now.
Would this also be pushed to the 3.3 branch? Or are there plans to make a
3.3.4 at some point? I saw a pipe-related hang reported to MSYS2 (that I
didn't see this issue in the stack traces), but I am not sure if there are
any more pipe fixes pending post 3.3.3.
More information about the Cygwin-patches
mailing list