[PATCH] stdio: fix vfscanf with matches longer than INT_MAX (bug 27650)

[Alyssa Ross]

>> I have not done a copyright assignment yet, but I think this change
>> should be small enough to be exempt?
>
> Yes, I think it's small enough.
>
> The test case wouldn't be, though.  I think the one on the bug needs
> some large (infinite) input on the stdin, though.  A real test case
> for glibc should probably involve pipe, fork, and fdopen.  fopencookie
> could work, too.

Oh, thanks for telling me about fopencookie!  I'd never have known about
that otherwise.  I've started having a go at a test case using it and it
seems like it'll work well.

>>  stdio-common/vfscanf-internal.c | 13 ++++---------
>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/stdio-common/vfscanf-internal.c b/stdio-common/vfscanf-internal.c
>> index 38e74776a5..1d81e16f4e 100644
>> --- a/stdio-common/vfscanf-internal.c
>> +++ b/stdio-common/vfscanf-internal.c
>> @@ -2479,11 +2479,6 @@ __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>>  	  else
>>  	    not_in = 0;
>>  
>> -	  if (width < 0)
>> -	    /* There is no width given so there is also no limit on the
>> -	       number of characters we read.  Therefore we set width to
>> -	       a very high value to make the algorithm easier.  */
>> -	    width = INT_MAX;
>>  
>>  #ifdef COMPILE_WSCANF
>>  	  /* Find the beginning and the end of the scanlist.  We are not
>> @@ -2647,7 +2642,7 @@ __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>>  			}
>>  		    }
>>  		}
>> -	      while (--width > 0 && inchar () != WEOF);
>> +	      while ((width < 0 || --width > 0) && inchar () != WEOF);
>>  	    out:
>>  #else
>>  	      char buf[MB_LEN_MAX];
>> @@ -2732,7 +2727,7 @@ __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>>  			}
>>  		    }
>>  
>> -		  if (--width <= 0)
>> +		  if (width >= 0 && --width <= 0)
>>  		    break;
>>  		}
>>  	      while (inchar () != EOF);
>> @@ -2884,7 +2879,7 @@ __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>>  		  assert (n <= MB_LEN_MAX);
>>  		  str += n;
>>  		}
>> -	      while (--width > 0 && inchar () != WEOF);
>> +	      while ((width < 0 || --width > 0) && inchar () != WEOF);
>>  	    out2:
>>  #else
>>  	      do
>> @@ -2938,7 +2933,7 @@ __vfscanf_internal (FILE *s, const char *format, va_list argptr,
>>  			}
>>  		    }
>>  		}
>> -	      while (--width > 0 && inchar () != EOF);
>> +	      while ((width < 0 || --width > 0) && inchar () != EOF);
>>  #endif
>>  
>>  	      if (__glibc_unlikely (now == read_in))
>
> So I tried to review this.  -U100 helped.  I was worried about width
> starting out as positive and going negative.  But as far as I can
> tell, processing stops once width == 0, so the issue cannot happen.

That's my understanding too.

> Do you want to work on the test case?  Will the copyright assignment
> be an obstacle?

I'm happy to have a go at a test case.  I think I have most of one
already.  Copyright assignment will only be a problem in that it'll slow
things down a bit!  I just sent one in for Emacs and I've asked about
doing one for glibc as well.

One question about the test: fscanf-ing through INT_MAX characters on a
trivial memcpy-based fopencookie stream takes 20 seconds on my
(admittedly fairly old) machine.  How slow is too slow for a test?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20210325/131e5830/attachment.sig>