Fix double free in __printf_fp_l (bug 26214)

[Adhemerval Zanella]

On 08/07/2020 15:01, Joseph Myers wrote:
> __printf_fp_l has a double free bug in the case where it allocates
> memory with malloc internally, then has an I/O error while outputting
> trailing padding and tries to free that already-freed memory when the
> error occurs.  This patch fixes this by setting the relevant pointer
> to NULL after the first free (the only free of this pointer that isn't
> immediately followed by returning from the function).
> 
> Tested for x86_64 and x86.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> index 73bf0da296..dc3fd38a19 100644
> --- a/stdio-common/Makefile
> +++ b/stdio-common/Makefile
> @@ -67,7 +67,8 @@ tests := tstscanf test_rdwr test-popen tstgetln test-fseek \
>  	 tst-renameat2 tst-bz11319 tst-bz11319-fortify2 \
>  	 scanf14a scanf16a \
>  	 tst-printf-bz25691 \
> -	 tst-vfprintf-width-prec-alloc
> +	 tst-vfprintf-width-prec-alloc \
> +	 tst-printf-fp-free
>  
>  
>  test-srcs = tst-unbputc tst-printf tst-printfsz-islongdouble

Ok.

> @@ -78,11 +79,13 @@ tests-special += $(objpfx)tst-unbputc.out $(objpfx)tst-printf.out \
>  		 $(objpfx)tst-setvbuf1-cmp.out \
>  		 $(objpfx)tst-vfprintf-width-prec-mem.out \
>  		 $(objpfx)tst-printfsz-islongdouble.out \
> -		 $(objpfx)tst-printf-bz25691-mem.out
> +		 $(objpfx)tst-printf-bz25691-mem.out \
> +		 $(objpfx)tst-printf-fp-free-mem.out
>  generated += tst-printf-bz18872.c tst-printf-bz18872.mtrace \
>  	     tst-printf-bz18872-mem.out \
>  	     tst-vfprintf-width-prec.mtrace tst-vfprintf-width-prec-mem.out \
> -	     tst-printf-bz25691.mtrace tst-printf-bz25691-mem.out
> +	     tst-printf-bz25691.mtrace tst-printf-bz25691-mem.out \
> +	     tst-printf-fp-free.mtrace tst-printf-fp-free-mem.out
>  endif
>  
>  tests-special += $(objpfx)tst-errno-manual.out
> @@ -108,6 +111,8 @@ tst-vfprintf-width-prec-ENV = \
>    MALLOC_TRACE=$(objpfx)tst-vfprintf-width-prec.mtrace
>  tst-printf-bz25691-ENV = \
>    MALLOC_TRACE=$(objpfx)tst-printf-bz25691.mtrace
> +tst-printf-fp-free-ENV = \
> +  MALLOC_TRACE=$(objpfx)tst-printf-fp-free.mtrace
>  
>  $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc
>  	$(SHELL) $< $(common-objpfx) '$(test-program-prefix)' > $@; \

Ok.

> diff --git a/stdio-common/printf_fp.c b/stdio-common/printf_fp.c
> index 9e0ce962f2..49c693575e 100644
> --- a/stdio-common/printf_fp.c
> +++ b/stdio-common/printf_fp.c
> @@ -1250,6 +1250,9 @@ __printf_fp_l (FILE *fp, locale_t loc,
>  	{
>  	  free (buffer);
>  	  free (wbuffer);
> +	  /* Avoid a double free if the subsequent PADN encounters an
> +	     I/O error.  */
> +	  wbuffer = NULL;
>  	}
>      }
>  

Ok.

> diff --git a/stdio-common/tst-printf-fp-free.c b/stdio-common/tst-printf-fp-free.c
> new file mode 100644
> index 0000000000..fea52248e0
> --- /dev/null
> +++ b/stdio-common/tst-printf-fp-free.c
> @@ -0,0 +1,37 @@
> +/* Test double free bug in __printf_fp_l (bug 26214).
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <mcheck.h>
> +#include <stdio.h>
> +#include <sys/resource.h>
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +{
> +  mtrace ();
> +  FILE *fp = fopen ("/dev/full", "w");
> +  TEST_VERIFY_EXIT (fp != NULL);
> +  char buf[131072];
> +  TEST_VERIFY_EXIT (setvbuf (fp, buf, _IOFBF, sizeof buf) == 0);
> +  TEST_COMPARE (fprintf (fp, "%-1000000.65536f", 1.0), -1);
> +  fclose (fp);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> 

Ok.