GNU bug report logs -
#6936
Bug#594666: /usr/bin/tac: tac aborts
Previous Next
Reported by: Jim Meyering <jim <at> meyering.net>
Date: Sat, 28 Aug 2010 16:25:02 UTC
Severity: normal
Done: Jim Meyering <jim <at> meyering.net>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
Your bug report
#6936: Bug#594666: /usr/bin/tac: tac aborts
which was filed against the coreutils package, has been closed.
The explanation is attached below, along with your original report.
If you require more details, please reply to 6936 <at> debbugs.gnu.org.
--
6936: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6936
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
Jim Meyering wrote:
> Salvo Tomaselli wrote:
>> Package: coreutils
>> Version: 8.5-1
>> Severity: normal
>> File: /usr/bin/tac
>>
>> Tac aborts when using it on a particular file.
>>
>> *** glibc detected *** tac: double free or corruption (top):
>> 0x00000000025c5030 ***
> ...
>
> Thank you for the report!
> That is indeed a bug in the very latest.
>
> For a stand-alone, minimal demonstrator, run this:
>
> valgrind tac <(printf %0$((2**14 + 1))d 0) > /dev/null
>
> It prints this:
>
> Invalid free() / delete / delete[]
> at 0x4A04D72: free (vg_replace_malloc.c:325)
> by 0x402294: main (tac.c:669)
> Address 0x4c30040 is 0 bytes inside a block of size 16,388 free'd
> at 0x4A05255: realloc (vg_replace_malloc.c:476)
> by 0x4117B8: xrealloc (xmalloc.c:57)
> by 0x401A68: tac_seekable (tac.c:319)
> by 0x402379: main (tac.c:515)
>
> Here is a fix:
>
>>From b3959fc691e606857a3c6e9b316ec34819972245 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering <at> redhat.com>
> Date: Sat, 28 Aug 2010 17:45:29 +0200
> Subject: [PATCH] tac: avoid double free
>
> * src/tac.c (main): Reading a line longer than 16KiB would cause
> tac to realloc its primary buffer. Then, just before exit, tac
> would mistakenly free the original (now free'd) buffer.
> This bug was introduced by commit be6c13e7, "maint: always free a
> buffer, to avoid even semblance of a leak".
> * NEWS (Bug fixes): Mention it.
> * tests/misc/tac (double-free): New test, to exercise this.
> Reported by Salvo Tomaselli in <http://bugs.debian.org/594666>.
This was fixed long ago.
Marking as "done".
[Message part 3 (message/rfc822, inline)]
Salvo Tomaselli wrote:
> Package: coreutils
> Version: 8.5-1
> Severity: normal
> File: /usr/bin/tac
>
> Tac aborts when using it on a particular file.
>
> *** glibc detected *** tac: double free or corruption (top): 0x00000000025c5030 ***
...
Thank you for the report!
That is indeed a bug in the very latest.
For a stand-alone, minimal demonstrator, run this:
valgrind tac <(printf %0$((2**14 + 1))d 0) > /dev/null
It prints this:
Invalid free() / delete / delete[]
at 0x4A04D72: free (vg_replace_malloc.c:325)
by 0x402294: main (tac.c:669)
Address 0x4c30040 is 0 bytes inside a block of size 16,388 free'd
at 0x4A05255: realloc (vg_replace_malloc.c:476)
by 0x4117B8: xrealloc (xmalloc.c:57)
by 0x401A68: tac_seekable (tac.c:319)
by 0x402379: main (tac.c:515)
Here is a fix:
From b3959fc691e606857a3c6e9b316ec34819972245 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Sat, 28 Aug 2010 17:45:29 +0200
Subject: [PATCH] tac: avoid double free
* src/tac.c (main): Reading a line longer than 16KiB would cause
tac to realloc its primary buffer. Then, just before exit, tac
would mistakenly free the original (now free'd) buffer.
This bug was introduced by commit be6c13e7, "maint: always free a
buffer, to avoid even semblance of a leak".
* NEWS (Bug fixes): Mention it.
* tests/misc/tac (double-free): New test, to exercise this.
Reported by Salvo Tomaselli in <http://bugs.debian.org/594666>.
---
NEWS | 3 +++
src/tac.c | 6 ++++--
tests/misc/tac | 6 ++++++
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index 85f55a2..f29d311 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ GNU coreutils NEWS -*- outline -*-
du -H and -L now consistently count pointed-to files instead of
symbolic links, and correctly diagnose dangling symlinks.
+ tac would perform a double-free when given an input line longer than 16KiB.
+ [bug introduced in coreutils-8.3]
+
** New features
cp now accepts the --attributes-only option to not copy file data,
diff --git a/src/tac.c b/src/tac.c
index cec9736..859e006 100644
--- a/src/tac.c
+++ b/src/tac.c
@@ -633,7 +633,6 @@ main (int argc, char **argv)
if (! (read_size < half_buffer_size && half_buffer_size < G_buffer_size))
xalloc_die ();
G_buffer = xmalloc (G_buffer_size);
- void *buf = G_buffer;
if (sentinel_length)
{
strcpy (G_buffer, separator);
@@ -666,6 +665,9 @@ main (int argc, char **argv)
error (0, errno, "-");
ok = false;
}
- free (buf);
+
+ size_t offset = sentinel_length ? sentinel_length : 1;
+ free (G_buffer - offset);
+
exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
}
diff --git a/tests/misc/tac b/tests/misc/tac
index 7631049..4130c00 100755
--- a/tests/misc/tac
+++ b/tests/misc/tac
@@ -24,6 +24,9 @@ my $prog = 'tac';
my $bad_dir = 'no/such/dir';
+# This must be longer than 16KiB to trigger the double free in coreutils-8.5.
+my $long_line = 'o' x (16 * 1024 + 1);
+
my @Tests =
(
['segfault', '-r', {IN=>"a\n"}, {IN=>"b\n"}, {OUT=>"a\nb\n"}],
@@ -67,6 +70,9 @@ my @Tests =
{ERR_SUBST => "s,`$bad_dir': .*,...,"},
{ERR => "$prog: cannot create temporary file in ...\n"},
{EXIT => 1}],
+
+ # coreutils-8.5's tac would double-free its primary buffer.
+ ['double-free', {IN=>$long_line}, {OUT=>$long_line}],
);
@Tests = triple_test \@Tests;
--
1.7.2.2.510.g7180a
This bug report was last modified 13 years and 294 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.