This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] libio: Free backup area when it not required (BZ#22415)


Ping.

On 29/11/2017 11:39, Adhemerval Zanella wrote:
> Some libio operations fail to correctly free the backup area (created
> by _IO_{w}default_pbackfail on unget{w}c) resulting in either invalid
> buffer free operations or memory leaks.
> 
> For instance, on the example provided by BZ#22415 a following
> fputc after a fseek to rewind the stream issues an invalid free on
> the buffer.  It is because although _IO_file_overflow correctly
> (from fputc) correctly calls _IO_free_backup_area, the
> _IO_new_file_seekoff (called by fseek) updates the FILE internal
> pointers without first free the backup area (resulting in invalid
> values in the internal pointers).
> 
> The wide version also shows an issue, but instead of accessing invalid
> pointers it leaks the backup memory on fseek/fputwc operation.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> 
> 	* libio/Makefile (tests): Add tst-bz22415.
> 	(tst-bz22415-ENV): New rule.
> 	(generated): Add tst-bz22415.mtrace and tst-bz22415.check.
> 	(tests-special): Add tst-bz22415-mem.out.
> 	($(objpfx)tst-bz22415-mem.out): New rule.
> 	* libio/fileops.c (_IO_new_file_seekoff): Call _IO_free_backup_area
> 	in case of a successful seek operation.
> 	* libio/wfileops.c (_IO_wfile_seekoff): Likewise.
> 	(_IO_wfile_overflow): Call _IO_free_wbackup_area in case a write
> 	buffer is required.
> 	* libio/tst-bz22415.c: New test.
> ---
>  ChangeLog           | 14 ++++++++
>  libio/Makefile      | 11 ++++--
>  libio/fileops.c     |  3 ++
>  libio/tst-bz22415.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libio/wfileops.c    |  4 +++
>  5 files changed, 127 insertions(+), 2 deletions(-)
>  create mode 100644 libio/tst-bz22415.c
> 
> diff --git a/libio/Makefile b/libio/Makefile
> index 9d09bd8..70b5530 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -62,7 +62,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
>  	bug-memstream1 bug-wmemstream1 \
>  	tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \
>  	tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \
> -	tst-ftell-append tst-fputws
> +	tst-ftell-append tst-fputws tst-bz22415
>  ifeq (yes,$(build-shared))
>  # Add test-fopenloc only if shared library is enabled since it depends on
>  # shared localedata objects.
> @@ -151,9 +151,11 @@ tst_wprintf2-ARGS = "Some Text"
>  
>  test-fmemopen-ENV = MALLOC_TRACE=$(objpfx)test-fmemopen.mtrace
>  tst-fopenloc-ENV = MALLOC_TRACE=$(objpfx)tst-fopenloc.mtrace
> +tst-bz22415-ENV = MALLOC_TRACE=$(objpfx)tst-bz22415.mtrace
>  
>  generated += test-fmemopen.mtrace test-fmemopen.check
>  generated += tst-fopenloc.mtrace tst-fopenloc.check
> +generated += tst-bz22415.mtrace tst-bz22415.check
>  
>  aux	:= fileops genops stdfiles stdio strops
>  
> @@ -167,7 +169,8 @@ shared-only-routines = oldiofopen oldiofdopen oldiofclose oldfileops	\
>  		       oldiofsetpos64
>  
>  ifeq ($(run-built-tests),yes)
> -tests-special += $(objpfx)test-freopen.out $(objpfx)test-fmemopen-mem.out
> +tests-special += $(objpfx)test-freopen.out $(objpfx)test-fmemopen-mem.out \
> +		 $(objpfx)tst-bz22415-mem.out
>  ifeq (yes,$(build-shared))
>  # Run tst-fopenloc-cmp.out and tst-openloc-mem.out only if shared
>  # library is enabled since they depend on tst-fopenloc.out.
> @@ -218,3 +221,7 @@ $(objpfx)test-fmemopen-mem.out: $(objpfx)test-fmemopen.out
>  $(objpfx)tst-fopenloc-mem.out: $(objpfx)tst-fopenloc.out
>  	$(common-objpfx)malloc/mtrace $(objpfx)tst-fopenloc.mtrace > $@; \
>  	$(evaluate-test)
> +
> +$(objpfx)tst-bz22415-mem.out: $(objpfx)tst-bz22415.out
> +	$(common-objpfx)malloc/mtrace $(objpfx)tst-bz22415.mtrace > $@; \
> +	$(evaluate-test)
> diff --git a/libio/fileops.c b/libio/fileops.c
> index a9e948f..72a0cb5 100644
> --- a/libio/fileops.c
> +++ b/libio/fileops.c
> @@ -994,6 +994,9 @@ _IO_new_file_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>  	  goto dumb;
>        }
>      }
> +
> +  _IO_free_backup_area (fp);
> +
>    /* At this point, dir==_IO_seek_set. */
>  
>    /* If destination is within current buffer, optimize: */
> diff --git a/libio/tst-bz22415.c b/libio/tst-bz22415.c
> new file mode 100644
> index 0000000..d7b23fe
> --- /dev/null
> +++ b/libio/tst-bz22415.c
> @@ -0,0 +1,97 @@
> +/* Check static buffer handling with setvbuf (BZ #22415)
> +
> +   Copyright (C) 2017 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <wchar.h>
> +#include <mcheck.h>
> +
> +#include <support/check.h>
> +#include <support/temp_file.h>
> +
> +static int
> +do_test (void)
> +{
> +  mtrace ();
> +
> +  char *temp_file;
> +  TEST_VERIFY_EXIT (create_temp_file ("tst-bz22145.", &temp_file));
> +
> +  char buf[BUFSIZ];
> +
> +  {
> +    /* Check if backup buffer is correctly freed and changing back
> +       to normal buffer does not trigger an invalid free in case of
> +       static buffer set by setvbuf.  */
> +
> +    FILE *f = fopen (temp_file, "w+b");
> +    TEST_VERIFY_EXIT (f != NULL);
> +
> +    TEST_VERIFY_EXIT (setvbuf (f, buf, _IOFBF, BUFSIZ) == 0);
> +    TEST_VERIFY_EXIT (ungetc ('x', f) == 'x');
> +    TEST_VERIFY_EXIT (fseek (f, 0L, SEEK_SET) == 0);
> +    TEST_VERIFY_EXIT (fputc ('y', f) ==  'y');
> +
> +    TEST_VERIFY_EXIT (fclose (f) == 0);
> +  }
> +
> +  {
> +    /* Check if backup buffer is correctly freed and changing back
> +       to normal buffer does not trigger an invalid free in case of
> +       static buffer set by setvbuf.  */
> +
> +    FILE *f = fopen (temp_file, "w+b");
> +    TEST_VERIFY_EXIT (f != NULL);
> +
> +    TEST_VERIFY_EXIT (setvbuf (f, buf, _IOFBF, BUFSIZ) == 0);
> +    TEST_VERIFY_EXIT (ungetc ('x', f) == 'x');
> +    TEST_VERIFY_EXIT (fputc ('y', f) ==  'y');
> +
> +    TEST_VERIFY_EXIT (fclose (f) == 0);
> +  }
> +
> +  {
> +    FILE *f = fopen (temp_file, "w+b");
> +    TEST_VERIFY_EXIT (f != NULL);
> +
> +    TEST_VERIFY_EXIT (setvbuf (f, buf, _IOFBF, BUFSIZ) == 0);
> +    TEST_VERIFY_EXIT (ungetwc (L'x', f) == L'x');
> +    TEST_VERIFY_EXIT (fseek (f, 0L, SEEK_SET) == 0);
> +    TEST_VERIFY_EXIT (fputwc (L'y', f) ==  L'y');
> +
> +    TEST_VERIFY_EXIT (fclose (f) == 0);
> +  }
> +
> +  {
> +    FILE *f = fopen (temp_file, "w+b");
> +    TEST_VERIFY_EXIT (f != NULL);
> +
> +    TEST_VERIFY_EXIT (setvbuf (f, buf, _IOFBF, BUFSIZ) == 0);
> +    TEST_VERIFY_EXIT (ungetwc (L'x', f) == L'x');
> +    TEST_VERIFY_EXIT (fputwc (L'y', f) ==  L'y');
> +
> +    TEST_VERIFY_EXIT (fclose (f) == 0);
> +  }
> +
> +  free (temp_file);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/libio/wfileops.c b/libio/wfileops.c
> index 8756b6f..91139e1 100644
> --- a/libio/wfileops.c
> +++ b/libio/wfileops.c
> @@ -421,6 +421,7 @@ _IO_wfile_overflow (_IO_FILE *f, wint_t wch)
>        if (f->_wide_data->_IO_write_base == 0)
>  	{
>  	  _IO_wdoallocbuf (f);
> +	  _IO_free_wbackup_area (f);
>  	  _IO_wsetg (f, f->_wide_data->_IO_buf_base,
>  		     f->_wide_data->_IO_buf_base, f->_wide_data->_IO_buf_base);
>  
> @@ -850,6 +851,9 @@ _IO_wfile_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>  	  goto dumb;
>        }
>      }
> +
> +  _IO_free_wbackup_area (fp);
> +
>    /* At this point, dir==_IO_seek_set. */
>  
>    /* If destination is within current buffer, optimize: */
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]