[PATCH] fhandler_pipe: add sanity limit to handle loops

[Ken Brown]

On 12/26/2021 5:43 PM, Jeremy Drake wrote:
> 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

Good idea, thanks.

>> +	     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.

Yes, I think so.

> 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.

There are some fixes (though not pipe-related) pending for 3.3.4, so I'll push 
it to the 3.3 branch after I've heard from Takashi and/or Corinna.

Ken