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

[Ken Brown <kbrown@cornell.edu>]

On 12/23/2021 7:29 PM, Jeremy Drake via Cygwin-patches wrote:
> On Thu, 23 Dec 2021, Ken Brown wrote:
> 
>>> -      for (ULONG j = 0; j < phi->NumberOfHandles; j++)
>>> +      for (ULONG j = 0; j < min(phi->NumberOfHandles, n_handle); j++)
>>
>> Reading the preceding code, I don't see how n_handle could be less than
>> phi->NumberOfHandles.  Can you explain?
>>
> 
> Not really.  I saw this stack trace:
> ...
> #3  0x0000000180062f13 in exception::handle (e=0x14cc4f0, frame=<optimized out>, in=<optimized out>, dispatch=<optimized out>) at /c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/exceptions.cc:835
> #4  0x00007ff8abd320cf in ntdll!.chkstk () from /c/Windows/SYSTEM32/ntdll.dll
> #5  0x00007ff8abce1454 in ntdll!RtlRaiseException () from /c/Windows/SYSTEM32/ntdll.dll
> #6  0x00007ff8abd30bfe in ntdll!KiUserExceptionDispatcher () from /c/Windows/SYSTEM32/ntdll.dll
> #7  0x0000000180092687 in fhandler_pipe::get_query_hdl_per_process (this=this@entry=0x1803700f8, name=name@entry=0x14cc820 L"\\Device\\NamedPipe\\dd50a72ab4668b33-10348-pipe-nt-0x6E6", ntfn=ntfn@entry=0x8000c2ce0) at /c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler_pipe.cc:1281
> #8  0x0000000180092bdb in fhandler_pipe::temporary_query_hdl (this=this@entry=0x1803700f8) at /c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler_pipe.cc:1190
> ...
> 
> Line 1281 of fhandler_pipe.cc was
> 	  if (phi->Handles[j].GrantedAccess != access)
> 
> The only way I could see that causing an exception was if it was reading
> past the end of the allocated memory, if j was greater than (or equal to)
> n_handle.  Unfortunately, I haven't been able to catch it in a debugger
> again, so I can't confirm this.  I took a core with 'dumper' but gdb
> doesn't want to load it (it says Core file format not supported, maybe
> something with msys2's gdb?).

I agree that it's hard to see how the line you quoted could cause an exception. 
  But you were using an optimized build, so I'm not sure how reliable the 
line-number information is.

Is it feasible to run your test under strace?  If so, you could add some 
debug_printf statements to examine the values of n_handle and 
phi->NumberOfHandles.  Or what about simply adding an assertion that 
phi->NumberOfHandles <= n_handle?

Ken