reference counting bug in swscanf et al

[Adhemerval Zanella Netto]

On 18/03/26 14:59, DJ Delorie wrote:
> 
> In swscanf.c we have this:
> 
>   FILE *f = _IO_strfile_readw (&sf, &wd, s);
> 
>   . . .
> 
>   return done;
> 
> _IO_strfile_readw calls:
> 
>   _IO_no_init (&sf->_sbf._f, _IO_USER_LOCK, 0, wd, &_IO_wstr_jumps);
>   _IO_fwide (&sf->_sbf._f, 1);
> 
> This eventually ends up in __wcsmbs_clone_conv(), where it
> reference-count-locks the shared object:
> 
>    bool overflow = false;
>   if (copy->towc->__shlib_handle != NULL)
>     overflow |= __builtin_add_overflow (copy->towc->__counter, 1,
>                                         &copy->towc->__counter);
>   if (copy->tomb->__shlib_handle != NULL)
>     overflow |= __builtin_add_overflow (copy->tomb->__counter, 1,
>                                         &copy->tomb->__counter);
> 
> However, the only place this refcount is released is in _IO_fclose(),
> where it does this:
> 
>   if (fp->_mode > 0)
>     {
>       /* This stream has a wide orientation.  This means we have to free
>          the conversion functions.  */
>       struct _IO_codecvt *cc = fp->_codecvt;
> 
>       __libc_lock_lock (__gconv_lock);
>       __gconv_release_step (cc->__cd_in.step);
>       __gconv_release_step (cc->__cd_out.step);
>       __libc_lock_unlock (__gconv_lock);
> 
> 
> in swscanf.c fclose is never called, nor can it be, since fclose also
> free's the FILE*, and in swscanf we're using a FILE struct on the
> stack.
> 
> Possible solutions?  First, do we need to refcount these still?  The
> reproducer works just fine other than the overflow.  Second, could we

My understanding is the refcount is still required because the 
__shlib_handle struct can be shared amount multiple threads and the
possible fix would to add _IO_new_fclose internal interface that expects
an stack-allocated FILE * (so no _IO_deallocate_file call).

> add some _IO_no_uninit() that can clean up the locks without free'ing
> the FILE* ?  Note that vswprintf() used to have this problem too, but
> its internals were completely replaced with a different algorith that
> doesn't.
> 
> 
> #include <stdio.h>
> #include <wchar.h>
> #include <locale.h>
> 
> int main(int argc, char *argv[])
> {
> 	long	i;
> 	int j;
> 	setlocale(LC_ALL, "en_US");
> 	wchar_t buf[32] = L"123";
> 	for (i = 0; i < 0x80000000; i++)
> 		swscanf(buf, L"%d", &j);
> 	return 0;
> }
>