public inbox for libc-stable@sourceware.org
 help / color / mirror / Atom feed
From: Siddhesh Poyarekar <siddhesh@sourceware.org>
To: libc-stable@sourceware.org
Cc: Carlos O'Donell <carlos@redhat.com>
Subject: [committed 2.40 5/5] ungetc: Fix backup buffer leak on program exit [BZ #27821]
Date: Fri, 30 Aug 2024 11:29:21 -0400	[thread overview]
Message-ID: <20240830152921.545802-6-siddhesh@sourceware.org> (raw)
In-Reply-To: <20240830152921.545802-1-siddhesh@sourceware.org>

If a file descriptor is left unclosed and is cleaned up by _IO_cleanup
on exit, its backup buffer remains unfreed, registering as a leak in
valgrind.  This is not strictly an issue since (1) the program should
ideally be closing the stream once it's not in use and (2) the program
is about to exit anyway, so keeping the backup buffer around a wee bit
longer isn't a real problem.  Free it anyway to keep valgrind happy
when the streams in question are the standard ones, i.e. stdout, stdin
or stderr.

Also, the _IO_have_backup macro checks for _IO_save_base,
which is a roundabout way to check for a backup buffer instead of
directly looking for _IO_backup_base.  The roundabout check breaks when
the main get area has not been used and user pushes a char into the
backup buffer with ungetc.  Fix this to use the _IO_backup_base
directly.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
(cherry picked from commit 3e1d8d1d1dca24ae90df2ea826a8916896fc7e77)
---
 libio/genops.c                 |  6 ++++++
 libio/libioP.h                 |  4 ++--
 stdio-common/Makefile          |  7 +++++++
 stdio-common/tst-ungetc-leak.c | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 stdio-common/tst-ungetc-leak.c

diff --git a/libio/genops.c b/libio/genops.c
index b012fa33d2..35d8b30710 100644
--- a/libio/genops.c
+++ b/libio/genops.c
@@ -816,6 +816,12 @@ _IO_unbuffer_all (void)
 	legacy = 1;
 #endif
 
+      /* Free up the backup area if it was ever allocated.  */
+      if (_IO_have_backup (fp))
+	_IO_free_backup_area (fp);
+      if (fp->_mode > 0 && _IO_have_wbackup (fp))
+	_IO_free_wbackup_area (fp);
+
       if (! (fp->_flags & _IO_UNBUFFERED)
 	  /* Iff stream is un-orientated, it wasn't used. */
 	  && (legacy || fp->_mode != 0))
diff --git a/libio/libioP.h b/libio/libioP.h
index 1af287b19f..616253fcd0 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -577,8 +577,8 @@ extern void _IO_old_init (FILE *fp, int flags) __THROW;
        ((__fp)->_wide_data->_IO_write_base \
 	= (__fp)->_wide_data->_IO_write_ptr = __p, \
 	(__fp)->_wide_data->_IO_write_end = (__ep))
-#define _IO_have_backup(fp) ((fp)->_IO_save_base != NULL)
-#define _IO_have_wbackup(fp) ((fp)->_wide_data->_IO_save_base != NULL)
+#define _IO_have_backup(fp) ((fp)->_IO_backup_base != NULL)
+#define _IO_have_wbackup(fp) ((fp)->_wide_data->_IO_backup_base != NULL)
 #define _IO_in_backup(fp) ((fp)->_flags & _IO_IN_BACKUP)
 #define _IO_have_markers(fp) ((fp)->_markers != NULL)
 #define _IO_blen(fp) ((fp)->_IO_buf_end - (fp)->_IO_buf_base)
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index e4f0146d2c..a91754f52d 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -254,6 +254,7 @@ tests := \
   tst-swscanf \
   tst-tmpnam \
   tst-ungetc \
+  tst-ungetc-leak \
   tst-unlockedio \
   tst-vfprintf-mbs-prec \
   tst-vfprintf-user-type \
@@ -316,6 +317,7 @@ tests-special += \
   $(objpfx)tst-printf-bz25691-mem.out \
   $(objpfx)tst-printf-fp-free-mem.out \
   $(objpfx)tst-printf-fp-leak-mem.out \
+  $(objpfx)tst-ungetc-leak-mem.out \
   $(objpfx)tst-vfprintf-width-prec-mem.out \
   # tests-special
 
@@ -330,6 +332,8 @@ generated += \
   tst-printf-fp-leak-mem.out \
   tst-printf-fp-leak.mtrace \
   tst-scanf-bz27650.mtrace \
+  tst-ungetc-leak-mem.out \
+  tst-ungetc-leak.mtrace \
   tst-vfprintf-width-prec-mem.out \
   tst-vfprintf-width-prec.mtrace \
   # generated
@@ -424,6 +428,9 @@ tst-printf-fp-leak-ENV = \
 tst-scanf-bz27650-ENV = \
   MALLOC_TRACE=$(objpfx)tst-scanf-bz27650.mtrace \
   LD_PRELOAD=$(common-objpfx)malloc/libc_malloc_debug.so
+tst-ungetc-leak-ENV = \
+  MALLOC_TRACE=$(objpfx)tst-ungetc-leak.mtrace \
+  LD_PRELOAD=$(common-objpfx)malloc/libc_malloc_debug.so
 
 $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc
 	$(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
diff --git a/stdio-common/tst-ungetc-leak.c b/stdio-common/tst-ungetc-leak.c
new file mode 100644
index 0000000000..6c5152b43f
--- /dev/null
+++ b/stdio-common/tst-ungetc-leak.c
@@ -0,0 +1,32 @@
+/* Test for memory leak with ungetc when stream is unused.
+   Copyright The GNU Toolchain Authors.
+   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 <stdio.h>
+#include <mcheck.h>
+#include <support/check.h>
+#include <support/support.h>
+
+static int
+do_test (void)
+{
+  mtrace ();
+  TEST_COMPARE (ungetc('y', stdin), 'y');
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.45.1


      parent reply	other threads:[~2024-08-30 15:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-30 15:29 [committed 2.40 0/5] vfscanf test case and ungetc fix Siddhesh Poyarekar
2024-08-30 15:29 ` [committed 2.40 1/5] support: Add FAIL test failure helper Siddhesh Poyarekar
2024-08-30 19:23   ` Siddhesh Poyarekar
2024-08-30 15:29 ` [committed 2.40 2/5] stdio-common: Add test for vfscanf with matches longer than INT_MAX [BZ #27650] Siddhesh Poyarekar
2024-08-30 15:29 ` [committed 2.40 3/5] Make tst-ungetc use libsupport Siddhesh Poyarekar
2024-08-30 15:29 ` [committed 2.40 4/5] ungetc: Fix uninitialized read when putting into unused streams [BZ #27821] Siddhesh Poyarekar
2024-08-30 15:29 ` Siddhesh Poyarekar [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240830152921.545802-6-siddhesh@sourceware.org \
    --to=siddhesh@sourceware.org \
    --cc=carlos@redhat.com \
    --cc=libc-stable@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).