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

[Jeremy Drake <cygwin@jdrake.com>]

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.