| 2012-03-15 08:34:58 |
Tom de Vries |
bug |
|
|
added bug |
| 2012-03-15 08:34:58 |
Tom de Vries |
attachment added |
|
guts.awk https://bugs.launchpad.net/bugs/955791/+attachment/2874155/+files/guts.awk |
|
| 2012-03-15 08:36:13 |
Tom de Vries |
attachment added |
|
CAT https://bugs.launchpad.net/ubuntu/+source/mawk/+bug/955791/+attachment/2874186/+files/CAT |
|
| 2012-03-17 20:16:16 |
Launchpad Janitor |
branch linked |
|
lp:~tjvries/ubuntu/natty/mawk/fix-for-955791 |
|
| 2012-03-22 18:47:23 |
Bryce Harrington |
description |
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
[Impact]
<fill me in with explanation of severity and frequency of bug on users and justification for backporting the fix to the stable release>
[Development Fix]
<fill me in with an explanation of how the bug has been addressed in the development branch, including the relevant version numbers of packages modified in order to implement the fix. >
[Stable Fix]
<fill me in by pointing out a minimal patch applicable to the stable version of the package.>
[Text Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
|
| 2012-03-22 20:34:38 |
Bryce Harrington |
description |
[Impact]
<fill me in with explanation of severity and frequency of bug on users and justification for backporting the fix to the stable release>
[Development Fix]
<fill me in with an explanation of how the bug has been addressed in the development branch, including the relevant version numbers of packages modified in order to implement the fix. >
[Stable Fix]
<fill me in by pointing out a minimal patch applicable to the stable version of the package.>
[Text Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
[Problem]
mawk loses data due to an improperly constructed memcpy() call.
[Impact]
Data loss in a data processing utility. Frequency of bugs affecting users is not known, but with the given test case occurs 100%.
[Development Fix]
Fixed upstream as of mawk-1.3.4-20100419 (TODO: Verify). We're carrying version 1.3.3-16ubuntu2 in precise. (TODO: Do we need to fix in precise as well?)
[Stable Fix]
lp:~tjvries/ubuntu/natty/mawk/fix-for-955791 is a debdiff for natty that applies this 1-liner change. (TODO: Does oneiric need the fix too?)
[Text Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
|
| 2012-03-22 21:30:37 |
Bryce Harrington |
mawk (Ubuntu): status |
New |
Triaged |
|
| 2012-03-22 21:30:41 |
Bryce Harrington |
mawk (Ubuntu): importance |
Undecided |
High |
|
| 2012-03-23 00:01:20 |
Bryce Harrington |
description |
[Problem]
mawk loses data due to an improperly constructed memcpy() call.
[Impact]
Data loss in a data processing utility. Frequency of bugs affecting users is not known, but with the given test case occurs 100%.
[Development Fix]
Fixed upstream as of mawk-1.3.4-20100419 (TODO: Verify). We're carrying version 1.3.3-16ubuntu2 in precise. (TODO: Do we need to fix in precise as well?)
[Stable Fix]
lp:~tjvries/ubuntu/natty/mawk/fix-for-955791 is a debdiff for natty that applies this 1-liner change. (TODO: Does oneiric need the fix too?)
[Text Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
[Problem]
mawk loses data due to an improperly constructed memcpy() call.
[Impact]
Data loss in a data processing utility. Frequency of bugs affecting users is not known, but with the given test case occurs 100%. lucid, maverick, natty, oneiric, and precise all carry mawk 1.3.3 and thus are affected.
[Development Fix]
Fixed upstream as of mawk-1.3.4-20100419 (TODO: Verify). We're carrying version 1.3.3-16ubuntu2 in precise. (TODO: Do we need to fix in precise as well?)
[Stable Fix]
lp:~tjvries/ubuntu/natty/mawk/fix-for-955791 is a debdiff for natty that applies this 1-liner change.
[Text Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
|
| 2012-03-23 00:33:23 |
Bryce Harrington |
description |
[Problem]
mawk loses data due to an improperly constructed memcpy() call.
[Impact]
Data loss in a data processing utility. Frequency of bugs affecting users is not known, but with the given test case occurs 100%. lucid, maverick, natty, oneiric, and precise all carry mawk 1.3.3 and thus are affected.
[Development Fix]
Fixed upstream as of mawk-1.3.4-20100419 (TODO: Verify). We're carrying version 1.3.3-16ubuntu2 in precise. (TODO: Do we need to fix in precise as well?)
[Stable Fix]
lp:~tjvries/ubuntu/natty/mawk/fix-for-955791 is a debdiff for natty that applies this 1-liner change.
[Text Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
[Problem]
mawk loses data due to an improperly constructed memcpy() call.
[Impact]
Data loss in a data processing utility. Frequency of bugs affecting users is not known, but with the given test case occurs 100%. lucid, maverick, natty, oneiric, and precise all carry mawk 1.3.3 and thus are affected.
[Development Fix]
Fixed upstream as of mawk-1.3.4-20100419. We're carrying version 1.3.3-16ubuntu2 in precise, which is affected by the bug. Same patch as is proposed in this SRU was uploaded to Precise March 22nd (Since we're frozen for beta2, it is delayed for archive admin review).
[Stable Fix]
lp:~tjvries/ubuntu/natty/mawk/fix-for-955791 is a debdiff for natty that applies this 1-liner change.
[Text Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
nominated for series |
|
Ubuntu Lucid |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
bug task added |
|
mawk (Ubuntu Lucid) |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
nominated for series |
|
Ubuntu Maverick |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
bug task added |
|
mawk (Ubuntu Maverick) |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
nominated for series |
|
Ubuntu Oneiric |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
bug task added |
|
mawk (Ubuntu Oneiric) |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
nominated for series |
|
Ubuntu Natty |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
bug task added |
|
mawk (Ubuntu Natty) |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
nominated for series |
|
Ubuntu Precise |
|
| 2012-03-23 00:42:51 |
Bryce Harrington |
bug task added |
|
mawk (Ubuntu Precise) |
|
| 2012-03-23 00:43:05 |
Bryce Harrington |
mawk (Ubuntu Oneiric): status |
New |
Fix Committed |
|
| 2012-03-23 00:43:08 |
Bryce Harrington |
mawk (Ubuntu Natty): status |
New |
Fix Committed |
|
| 2012-03-23 00:43:11 |
Bryce Harrington |
mawk (Ubuntu Maverick): status |
New |
Fix Committed |
|
| 2012-03-23 00:43:15 |
Bryce Harrington |
mawk (Ubuntu Lucid): status |
New |
Fix Committed |
|
| 2012-03-23 00:50:28 |
Bryce Harrington |
mawk (Ubuntu Precise): status |
Triaged |
Fix Committed |
|
| 2012-03-23 02:23:28 |
Kate Stewart |
tags |
|
rls-mgr-p-tracking |
|
| 2012-03-23 06:36:07 |
Launchpad Janitor |
mawk (Ubuntu Precise): status |
Fix Committed |
Fix Released |
|
| 2012-03-23 07:07:19 |
Launchpad Janitor |
branch linked |
|
lp:ubuntu/mawk |
|
| 2012-03-23 07:49:42 |
Martin Pitt |
bug |
|
|
added subscriber Ubuntu Stable Release Updates Team |
| 2012-03-23 07:49:43 |
Martin Pitt |
bug |
|
|
added subscriber SRU Verification |
| 2012-03-23 07:49:48 |
Martin Pitt |
tags |
rls-mgr-p-tracking |
rls-mgr-p-tracking verification-needed |
|
| 2012-03-23 07:51:33 |
Martin Pitt |
mawk (Ubuntu Lucid): status |
Fix Committed |
In Progress |
|
| 2012-03-23 07:52:17 |
Launchpad Janitor |
branch linked |
|
lp:ubuntu/natty-proposed/mawk |
|
| 2012-03-23 07:53:35 |
Martin Pitt |
mawk (Ubuntu Maverick): status |
Fix Committed |
Won't Fix |
|
| 2012-03-23 07:53:39 |
Martin Pitt |
mawk (Ubuntu Oneiric): status |
Fix Committed |
In Progress |
|
| 2012-03-23 17:39:11 |
Bryce Harrington |
mawk (Ubuntu Lucid): status |
In Progress |
Fix Committed |
|
| 2012-03-23 17:39:16 |
Bryce Harrington |
mawk (Ubuntu Oneiric): status |
In Progress |
Fix Committed |
|
| 2012-03-24 09:40:24 |
Launchpad Janitor |
branch linked |
|
lp:debian/mawk |
|
| 2012-03-27 11:13:19 |
Launchpad Janitor |
branch linked |
|
lp:ubuntu/oneiric-proposed/mawk |
|
| 2012-03-28 05:21:13 |
Martin Pitt |
tags |
rls-mgr-p-tracking verification-needed |
rls-mgr-p-tracking verification-done-oneiric verification-needed |
|
| 2012-03-29 07:13:20 |
Launchpad Janitor |
branch linked |
|
lp:ubuntu/lucid-proposed/mawk |
|
| 2012-04-25 08:50:15 |
Launchpad Janitor |
mawk (Ubuntu Oneiric): status |
Fix Committed |
Fix Released |
|
| 2012-08-01 17:23:07 |
Clint Byrum |
description |
[Problem]
mawk loses data due to an improperly constructed memcpy() call.
[Impact]
Data loss in a data processing utility. Frequency of bugs affecting users is not known, but with the given test case occurs 100%. lucid, maverick, natty, oneiric, and precise all carry mawk 1.3.3 and thus are affected.
[Development Fix]
Fixed upstream as of mawk-1.3.4-20100419. We're carrying version 1.3.3-16ubuntu2 in precise, which is affected by the bug. Same patch as is proposed in this SRU was uploaded to Precise March 22nd (Since we're frozen for beta2, it is delayed for archive admin review).
[Stable Fix]
lp:~tjvries/ubuntu/natty/mawk/fix-for-955791 is a debdiff for natty that applies this 1-liner change.
[Text Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
[Problem]
mawk loses data due to an improperly constructed memcpy() call.
[Impact]
Data loss in a data processing utility. Frequency of bugs affecting users is not known, but with the given test case occurs 100%. lucid, maverick, natty, oneiric, and precise all carry mawk 1.3.3 and thus are affected.
[Development Fix]
Fixed upstream as of mawk-1.3.4-20100419. We're carrying version 1.3.3-16ubuntu2 in precise, which is affected by the bug. Same patch as is proposed in this SRU was uploaded to Precise March 22nd (Since we're frozen for beta2, it is delayed for archive admin review).
[Stable Fix]
lp:~tjvries/ubuntu/natty/mawk/fix-for-955791 is a debdiff for natty that applies this 1-liner change.
[Test Case]
<fill me in with detailed *instructions* on how to reproduce the bug. This will be used by people later on to verify the updated package fixes the problem.>
1.
2.
3.
Broken Behavior:
Fixed Behavior:
[Regression Potential]
<fill me in with a discussion of likelihood and potential severity of regressions and how users could get inadvertently affected.
[Original Report]
Source package: mawk
release: 11.10 x86_64
version: 1.3.3-15ubuntu2
on 11.10, I run this series of commands. Both greps are supposed to return 4.
...
$ grep -c 'abi/mangle33.C.*scan' CAT | grep -v ':0'
4
$ rm -f list[0-9]*
$ cat CAT | mawk -f guts.awk
$ grep -c 'abi/mangle33.C.*scan' list* | grep -v ':0'
3
....
This doesn't reproduce on 10.10, there indeed the last grep returns 4.
Running valgrind on the mawk invocation reveals a problem:
...
$ cat CAT | valgrind mawk -f guts.awk
==8659== Memcheck, a memory error detector
==8659== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==8659== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==8659== Command: mawk -f guts.awk
==8659==
Running target unix
==8659== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==8659== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==8659== by 0x410FC7: ??? (in /usr/bin/mawk)
==8659== by 0x406BA5: ??? (in /usr/bin/mawk)
==8659== by 0x405F99: ??? (in /usr/bin/mawk)
==8659== by 0x50D430C: (below main) (libc-start.c:226)
==8659==
...
And that reproduces on 10.10 as well.
I recompiled the source package at -O0 with -g3, and reran valgrind:
...
$ cat CAT | valgrind mawk/mawk-1.3.3/mawk -f guts.awk
==9007== Memcheck, a memory error detector
==9007== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==9007== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright info
==9007== Command: mawk/mawk-1.3.3/mawk -f guts.awk
==9007==
Running target unix
==9007== Source and destination overlap in memcpy(0x545dd90, 0x545e569, 2087)
==9007== at 0x4C2A1C4: memcpy (mc_replace_strmem.c:635)
==9007== by 0x41465E: FINgets (fin.c:329)
==9007== by 0x409674: execute (execute.c:1116)
==9007== by 0x406F8A: main (main.c:66)
==9007==
...
That gives me the violating memcpy:
...
if (p == fin->buff)
{
/* current record is too big for the input buffer, grow buffer */
p = enlarge_fin_buffer(fin) ;
}
else
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
fin->flags |= EOF_FLAG ;
}
...
Using the following patch, I abort when the error condition is detected, and now I can reproduce on both 11.10 and 10.10 without valgrind, and easily hit the error in a gdb session.
...
--- fin.c 2012-03-15 09:03:59.674854168 +0100
+++ fin.c.fixed 2012-03-15 09:02:32.910857237 +0100
@@ -76,6 +76,7 @@
#include "field.h"
#include "symtype.h"
#include "scan.h"
+#include <stdlib.h>
#ifndef NO_FCNTL_H
#include <fcntl.h>
@@ -325,8 +326,19 @@
{
/* move a partial line to front of buffer and try again */
unsigned rr ;
-
- p = (char *) memcpy(fin->buff, p, r = strlen(p)) ;
+ char *src, *dst, *srclast, *dstlast;
+ dst = fin->buff;
+ src = p;
+ r = strlen(p);
+ srclast = src + r - 1;
+ dstlast = dst + r - 1;
+ if (!(srclast < dst || dstlast < src))
+ {
+ fprintf (stderr, "overlapping memcpy (%lx, %lx, %u)\n", dst, src, r);
+ fflush (stderr);
+ abort ();
+ }
+ p = (char *)memcpy (fin->buff, p, r) ;
q = p+r ; rr = fin->nbuffs*BUFFSZ - r ;
if ((r = fillbuff(fin->fd, q, rr)) < rr)
...
Replacing the memcpy with a memmove fixes the problem, and I think it's a sufficient fix. |
|
| 2012-11-16 17:28:11 |
Brian Murray |
tags |
rls-mgr-p-tracking verification-done-oneiric verification-needed |
removal-candidate rls-mgr-p-tracking verification-done-oneiric verification-needed |
|
| 2012-11-30 19:55:10 |
Brian Murray |
mawk (Ubuntu Natty): status |
Fix Committed |
Won't Fix |
|
| 2012-11-30 20:17:18 |
Brian Murray |
mawk (Ubuntu Lucid): status |
Fix Committed |
Triaged |
|
| 2012-11-30 20:19:18 |
Brian Murray |
tags |
removal-candidate rls-mgr-p-tracking verification-done-oneiric verification-needed |
removal-candidate rls-mgr-p-tracking verification-done-oneiric |
|
| 2012-11-30 20:19:19 |
Brian Murray |
tags |
removal-candidate rls-mgr-p-tracking verification-done-oneiric |
rls-mgr-p-tracking verification-done-oneiric |
|
| 2015-06-17 12:16:54 |
Rolf Leggewie |
mawk (Ubuntu Lucid): status |
Triaged |
Won't Fix |
|
