From 2791e9e3ecff8bbbb80d3eb05fa0db4450e44e9f4010e06a51b725ccb1644d92 Mon Sep 17 00:00:00 2001 From: Philipp Thomas Date: Mon, 17 Oct 2011 13:29:22 +0000 Subject: [PATCH 1/2] - Add upstream patch that fixes three bugs in tac: - remove sole use of sprintf in favor of stpcpy - don't misbehave with multiple non-seekable inputs - don't leak a file descriptor for each non-seekable input OBS-URL: https://build.opensuse.org/package/show/Base:System/coreutils?expand=0&rev=132 --- coreutils-fix_tac.patch | 223 ++++++++++++++++++++++++++++++++++++++++ coreutils.changes | 8 ++ coreutils.spec | 2 + 3 files changed, 233 insertions(+) create mode 100644 coreutils-fix_tac.patch diff --git a/coreutils-fix_tac.patch b/coreutils-fix_tac.patch new file mode 100644 index 0000000..c30c0ec --- /dev/null +++ b/coreutils-fix_tac.patch @@ -0,0 +1,223 @@ +>From cdd328f232a93fb40aec25d0681ef191eaeba2da Mon Sep 17 00:00:00 2001 +From: Jim Meyering +Date: Sun, 16 Oct 2011 10:35:56 +0200 +Subject: [PATCH 1/3] maint: tac: remove sole use of sprintf in favor of + stpcpy + +* src/tac.c (copy_to_temp): Use stpcpy rather than sprintf. +Move some declarations "down" to point of initialization. +--- + src/tac.c | 17 +++++++---------- + 1 files changed, 7 insertions(+), 10 deletions(-) + +diff --git a/src/tac.c b/src/tac.c +index 65ac6a6..c572862 100644 +--- src/tac.c ++++ src/tac.c +@@ -426,20 +426,17 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) + { + static char *template = NULL; + static char const *tempdir; +- char *tempfile; +- FILE *tmp; +- int fd; + + if (template == NULL) + { +- char const * const Template = "%s/tacXXXXXX"; ++ char const * const Template = "tacXXXXXX"; + tempdir = getenv ("TMPDIR"); + if (tempdir == NULL) + tempdir = DEFAULT_TMPDIR; + +- /* Subtract 2 for `%s' and add 1 for the trailing NUL byte. */ +- template = xmalloc (strlen (tempdir) + strlen (Template) - 2 + 1); +- sprintf (template, Template, tempdir); ++ /* Add 1 for the slash and one for the trailing NUL byte. */ ++ template = xmalloc (strlen (tempdir) + strlen (Template) + 1 + 1); ++ stpcpy (stpcpy (stpcpy (template, tempdir), "/"), Template); + } + + /* FIXME: there's a small window between a successful mkstemp call +@@ -451,8 +448,8 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) + FIXME: clean up upon fatal signal. Don't block them, in case + $TMPFILE is a remote file system. */ + +- tempfile = template; +- fd = mkstemp (template); ++ char *tempfile = template; ++ int fd = mkstemp (template); + if (fd < 0) + { + error (0, errno, _("cannot create temporary file in %s"), +@@ -460,7 +457,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) + return false; + } + +- tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); ++ FILE *tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); + if (! tmp) + { + error (0, errno, _("cannot open %s for writing"), quote (tempfile)); +-- +1.7.7 + + +>From 608f9d9d0daef48c957fe38570e5d0f293c0f1eb Mon Sep 17 00:00:00 2001 +From: Jim Meyering +Date: Sun, 16 Oct 2011 12:07:05 +0200 +Subject: [PATCH 2/3] tac: don't misbehave with multiple non-seekable inputs + +* src/tac.c (copy_to_temp): Do not reuse the template buffer. +Instead, scribble only on a freshly-xstrdup'd copy each time. +Free that buffer both here, upon failure, and ... +(tac_nonseekable): ...free the buffer in caller, upon success. +* tests/misc/tac-2-nonseekable: New file. +* tests/Makefile.am (TESTS): Add it. +* NEWS (Bug fixes): Mention it. +Reported by Ambrose Feinstein in http://debbugs.gnu.org/9762. +--- + NEWS | 5 +++++ + src/tac.c | 19 +++++++++++++++---- + tests/Makefile.am | 1 + + tests/misc/tac-2-nonseekable | 27 +++++++++++++++++++++++++++ + 4 files changed, 48 insertions(+), 4 deletions(-) + create mode 100755 tests/misc/tac-2-nonseekable + +diff --git a/src/tac.c b/src/tac.c +index c572862..2d8d6ea 100644 +--- src/tac.c ++++ src/tac.c +@@ -448,12 +448,13 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) + FIXME: clean up upon fatal signal. Don't block them, in case + $TMPFILE is a remote file system. */ + +- char *tempfile = template; +- int fd = mkstemp (template); ++ char *tempfile = xstrdup (template); ++ int fd = mkstemp (tempfile); + if (fd < 0) + { + error (0, errno, _("cannot create temporary file in %s"), + quote (tempdir)); ++ free (tempfile); + return false; + } + +@@ -463,6 +464,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) + error (0, errno, _("cannot open %s for writing"), quote (tempfile)); + close (fd); + unlink (tempfile); ++ free (tempfile); + return false; + } + +@@ -498,6 +500,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) + + Fail: + fclose (tmp); ++ free (tempfile); + return false; + } + +@@ -509,8 +512,16 @@ tac_nonseekable (int input_fd, const char *file) + { + FILE *tmp_stream; + char *tmp_file; +- return (copy_to_temp (&tmp_stream, &tmp_file, input_fd, file) +- && tac_seekable (fileno (tmp_stream), tmp_file)); ++ if (copy_to_temp (&tmp_stream, &tmp_file, input_fd, file)) ++ { ++ if (tac_seekable (fileno (tmp_stream), tmp_file)) ++ { ++ free (tmp_file); ++ return true; ++ } ++ } ++ ++ return false; + } + + /* Print FILE in reverse, copying it to a temporary +diff --git a/tests/Makefile.am b/tests/Makefile.am +index 9c9a1b8..5021c18 100644 +--- tests/Makefile.am ++++ tests/Makefile.am +@@ -274,6 +274,7 @@ TESTS = \ + misc/sum-sysv \ + misc/tac \ + misc/tac-continue \ ++ misc/tac-2-nonseekable \ + misc/tail \ + misc/tee \ + misc/tee-dash \ +diff --git a/tests/misc/tac-2-nonseekable b/tests/misc/tac-2-nonseekable +new file mode 100755 +index 0000000..7b48773 +--- /dev/null ++++ tests/misc/tac-2-nonseekable +@@ -0,0 +1,27 @@ ++#!/bin/sh ++# ensure that tac works with two or more non-seekable inputs ++ ++# Copyright (C) 2011 Free Software Foundation, Inc. ++ ++# This program is free software: you can redistribute it and/or modify ++# it under the terms of the GNU General Public License as published by ++# the Free Software Foundation, either version 3 of the License, or ++# (at your option) any later version. ++ ++# This program 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 General Public License for more details. ++ ++# You should have received a copy of the GNU General Public License ++# along with this program. If not, see . ++ ++. "${srcdir=.}/init.sh"; path_prepend_ ../src ++print_ver_ tac ++ ++echo x | tac - - > out 2> err || fail=1 ++echo x > exp || fail=1 ++compare out exp || fail=1 ++compare err /dev/null || fail=1 ++ ++Exit $fail +-- +1.7.7 + + +>From 95fa11b63f0a5c983723f02afa7c5b896e5a0a97 Mon Sep 17 00:00:00 2001 +From: Jim Meyering +Date: Sun, 16 Oct 2011 12:14:05 +0200 +Subject: [PATCH 3/3] tac: don't leak a file descriptor for each non-seekable + input + +* src/tac.c (tac_nonseekable): Call fclose after each successful +call to copy_to_temp. +--- + src/tac.c | 2 ++ + 1 files changed, 2 insertions(+), 0 deletions(-) + +diff --git a/src/tac.c b/src/tac.c +index 2d8d6ea..ebafa18 100644 +--- src/tac.c ++++ src/tac.c +@@ -516,9 +516,11 @@ tac_nonseekable (int input_fd, const char *file) + { + if (tac_seekable (fileno (tmp_stream), tmp_file)) + { ++ fclose (tmp_stream); + free (tmp_file); + return true; + } ++ fclose (tmp_stream); + } + + return false; +-- +1.7.7 + + + diff --git a/coreutils.changes b/coreutils.changes index 60aa3a5..e1ed2b9 100644 --- a/coreutils.changes +++ b/coreutils.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Mon Oct 17 15:25:21 CEST 2011 - pth@suse.de + +- Add upstream patch that fixes three bugs in tac: + - remove sole use of sprintf in favor of stpcpy + - don't misbehave with multiple non-seekable inputs + - don't leak a file descriptor for each non-seekable input + ------------------------------------------------------------------- Fri Oct 14 16:51:48 CEST 2011 - pth@suse.de diff --git a/coreutils.spec b/coreutils.spec index 1475b7c..8d2dffb 100644 --- a/coreutils.spec +++ b/coreutils.spec @@ -56,6 +56,7 @@ Patch31: coreutils-getaddrinfo.patch Patch32: coreutils-ptr_int_casts.patch Patch33: coreutils-8.9-singlethreaded-sort.patch Patch34: coreutils-acl-nofollow.patch +Patch35: coreutils-fix_tac.patch BuildRoot: %{_tmppath}/%{name}-%{version}-build # this will create a cycle, broken up randomly - coreutils is just too core to have other # prerequires @@ -97,6 +98,7 @@ uname unexpand uniq unlink uptime users vdir wc who whoami yes %patch32 %patch33 %patch34 +%patch35 xz -dc %{S:4} >po/de.po From 98b9b6674010684c506c27760951e7861e020bc35103af413a0d82e85a3f802b Mon Sep 17 00:00:00 2001 From: Philipp Thomas Date: Mon, 17 Oct 2011 14:02:20 +0000 Subject: [PATCH 2/2] - Fix the fd leak in error path. OBS-URL: https://build.opensuse.org/package/show/Base:System/coreutils?expand=0&rev=133 --- coreutils-fix_tac.patch | 160 ++++++++++------------------------------ 1 file changed, 37 insertions(+), 123 deletions(-) diff --git a/coreutils-fix_tac.patch b/coreutils-fix_tac.patch index c30c0ec..82dae15 100644 --- a/coreutils-fix_tac.patch +++ b/coreutils-fix_tac.patch @@ -10,18 +10,18 @@ Move some declarations "down" to point of initialization. src/tac.c | 17 +++++++---------- 1 files changed, 7 insertions(+), 10 deletions(-) -diff --git a/src/tac.c b/src/tac.c -index 65ac6a6..c572862 100644 ---- src/tac.c -+++ src/tac.c -@@ -426,20 +426,17 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) +Index: src/tac.c +=================================================================== +--- src/tac.c.orig 2011-02-19 18:17:03.000000000 +0100 ++++ src/tac.c 2011-10-17 15:46:27.879485098 +0200 +@@ -426,20 +426,17 @@ copy_to_temp (FILE **g_tmp, char **g_tem { static char *template = NULL; static char const *tempdir; - char *tempfile; - FILE *tmp; - int fd; - + if (template == NULL) { - char const * const Template = "%s/tacXXXXXX"; @@ -29,7 +29,7 @@ index 65ac6a6..c572862 100644 tempdir = getenv ("TMPDIR"); if (tempdir == NULL) tempdir = DEFAULT_TMPDIR; - + - /* Subtract 2 for `%s' and add 1 for the trailing NUL byte. */ - template = xmalloc (strlen (tempdir) + strlen (Template) - 2 + 1); - sprintf (template, Template, tempdir); @@ -37,63 +37,14 @@ index 65ac6a6..c572862 100644 + template = xmalloc (strlen (tempdir) + strlen (Template) + 1 + 1); + stpcpy (stpcpy (stpcpy (template, tempdir), "/"), Template); } - + /* FIXME: there's a small window between a successful mkstemp call -@@ -451,8 +448,8 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) +@@ -451,21 +448,23 @@ copy_to_temp (FILE **g_tmp, char **g_tem FIXME: clean up upon fatal signal. Don't block them, in case $TMPFILE is a remote file system. */ - + - tempfile = template; - fd = mkstemp (template); -+ char *tempfile = template; -+ int fd = mkstemp (template); - if (fd < 0) - { - error (0, errno, _("cannot create temporary file in %s"), -@@ -460,7 +457,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) - return false; - } - -- tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); -+ FILE *tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); - if (! tmp) - { - error (0, errno, _("cannot open %s for writing"), quote (tempfile)); --- -1.7.7 - - ->From 608f9d9d0daef48c957fe38570e5d0f293c0f1eb Mon Sep 17 00:00:00 2001 -From: Jim Meyering -Date: Sun, 16 Oct 2011 12:07:05 +0200 -Subject: [PATCH 2/3] tac: don't misbehave with multiple non-seekable inputs - -* src/tac.c (copy_to_temp): Do not reuse the template buffer. -Instead, scribble only on a freshly-xstrdup'd copy each time. -Free that buffer both here, upon failure, and ... -(tac_nonseekable): ...free the buffer in caller, upon success. -* tests/misc/tac-2-nonseekable: New file. -* tests/Makefile.am (TESTS): Add it. -* NEWS (Bug fixes): Mention it. -Reported by Ambrose Feinstein in http://debbugs.gnu.org/9762. ---- - NEWS | 5 +++++ - src/tac.c | 19 +++++++++++++++---- - tests/Makefile.am | 1 + - tests/misc/tac-2-nonseekable | 27 +++++++++++++++++++++++++++ - 4 files changed, 48 insertions(+), 4 deletions(-) - create mode 100755 tests/misc/tac-2-nonseekable - -diff --git a/src/tac.c b/src/tac.c -index c572862..2d8d6ea 100644 ---- src/tac.c -+++ src/tac.c -@@ -448,12 +448,13 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) - FIXME: clean up upon fatal signal. Don't block them, in case - $TMPFILE is a remote file system. */ - -- char *tempfile = template; -- int fd = mkstemp (template); + char *tempfile = xstrdup (template); + int fd = mkstemp (tempfile); if (fd < 0) @@ -103,47 +54,48 @@ index c572862..2d8d6ea 100644 + free (tempfile); return false; } - -@@ -463,6 +464,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) + +- tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); ++ FILE *tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+")); + if (! tmp) + { error (0, errno, _("cannot open %s for writing"), quote (tempfile)); close (fd); unlink (tempfile); + free (tempfile); return false; } - -@@ -498,6 +500,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file) - + +@@ -501,6 +500,7 @@ copy_to_temp (FILE **g_tmp, char **g_tem + Fail: fclose (tmp); + free (tempfile); return false; } - -@@ -509,8 +512,16 @@ tac_nonseekable (int input_fd, const char *file) + +@@ -512,8 +512,14 @@ tac_nonseekable (int input_fd, const cha { FILE *tmp_stream; char *tmp_file; - return (copy_to_temp (&tmp_stream, &tmp_file, input_fd, file) - && tac_seekable (fileno (tmp_stream), tmp_file)); -+ if (copy_to_temp (&tmp_stream, &tmp_file, input_fd, file)) -+ { -+ if (tac_seekable (fileno (tmp_stream), tmp_file)) -+ { -+ free (tmp_file); -+ return true; -+ } -+ } + -+ return false; ++ if (!copy_to_temp (&tmp_stream, &tmp_file, input_fd, file)) ++ return false; ++ ++ bool ok = tac_seekable (fileno (tmp_stream), tmp_file); ++ fclose (tmp_stream); ++ free (tmp_file); ++ return ok; } - + /* Print FILE in reverse, copying it to a temporary -diff --git a/tests/Makefile.am b/tests/Makefile.am -index 9c9a1b8..5021c18 100644 ---- tests/Makefile.am -+++ tests/Makefile.am -@@ -274,6 +274,7 @@ TESTS = \ +Index: tests/Makefile.am +=================================================================== +--- tests/Makefile.am.orig 2011-10-17 15:40:44.533154336 +0200 ++++ tests/Makefile.am 2011-10-17 15:40:44.882149592 +0200 +@@ -270,6 +270,7 @@ TESTS = \ misc/sum-sysv \ misc/tac \ misc/tac-continue \ @@ -151,11 +103,10 @@ index 9c9a1b8..5021c18 100644 misc/tail \ misc/tee \ misc/tee-dash \ -diff --git a/tests/misc/tac-2-nonseekable b/tests/misc/tac-2-nonseekable -new file mode 100755 -index 0000000..7b48773 ---- /dev/null -+++ tests/misc/tac-2-nonseekable +Index: tests/misc/tac-2-nonseekable +=================================================================== +--- /dev/null 1970-01-01 00:00:00.000000000 +0000 ++++ tests/misc/tac-2-nonseekable 2011-10-17 15:40:44.883149578 +0200 @@ -0,0 +1,27 @@ +#!/bin/sh +# ensure that tac works with two or more non-seekable inputs @@ -184,40 +135,3 @@ index 0000000..7b48773 +compare err /dev/null || fail=1 + +Exit $fail --- -1.7.7 - - ->From 95fa11b63f0a5c983723f02afa7c5b896e5a0a97 Mon Sep 17 00:00:00 2001 -From: Jim Meyering -Date: Sun, 16 Oct 2011 12:14:05 +0200 -Subject: [PATCH 3/3] tac: don't leak a file descriptor for each non-seekable - input - -* src/tac.c (tac_nonseekable): Call fclose after each successful -call to copy_to_temp. ---- - src/tac.c | 2 ++ - 1 files changed, 2 insertions(+), 0 deletions(-) - -diff --git a/src/tac.c b/src/tac.c -index 2d8d6ea..ebafa18 100644 ---- src/tac.c -+++ src/tac.c -@@ -516,9 +516,11 @@ tac_nonseekable (int input_fd, const char *file) - { - if (tac_seekable (fileno (tmp_stream), tmp_file)) - { -+ fclose (tmp_stream); - free (tmp_file); - return true; - } -+ fclose (tmp_stream); - } - - return false; --- -1.7.7 - - -