Debian Bug report logs - #347695
bash: segfault in "while read" loop

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Laird Breyer <laird@lbreyer.com>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: bash: segfault in "while read" loop

Date: Thu, 12 Jan 2006 16:45:53 +1100

Package: bash
Version: 3.1-1
Severity: normal

The following command segfaults at line 4097:

% seq 0 10000 | while read line; do /bin/echo $line ; done | tail -2
4095
4096

The seq, echo and tail commands are for illustration only, they are
not related to the bug. I originally discovered the bug with completely
different commands, but the above is the simplest illustration of the bug.

The segfault appears when the input reaches line 4097, and when an
external command is invoked. 
If you use the builtin echo, the bug doesn't appear:

% seq 0 10000 | while read line; do echo $line ; done | tail -2
9999
10000
   



-- System Information:
Debian Release: testing/unstable
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.14-2-686
Locale: LANG=en_AU, LC_CTYPE=en_AU (charmap=ISO-8859-1)

Versions of packages bash depends on:
ii  base-files                    3.1.9      Debian base system miscellaneous f
ii  debianutils                   2.15.2     Miscellaneous utilities specific t
ii  libc6                         2.3.5-8    GNU C Library: Shared libraries an
ii  libncurses5                   5.5-1      Shared libraries for terminal hand

bash recommends no packages.

-- no debconf information

Message #10 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Justin Pryzby <justinpryzby@users.sourceforge.net>

To: laird@lbreyer.com, 347695@bugs.debian.org

Subject: Re: Bug#347695: bash: segfault in "while read" loop

Date: Thu, 12 Jan 2006 09:15:19 -0500

tag 347695 confirmed
thanks

You can get the 'segmentation fault' message with the command:

  seq 0 10000 | while read line; do /bin/echo $line; done >tmpout

And here is the stack trace for Debian's bash3.1-2:

#0  0x0807b851 in kill_pid ()
#1  0x0807bdf7 in wait_for ()
#2  0x0806dcaf in execute_command_internal ()
#3  0x0806f9b0 in execute_command ()
#4  0x080700c0 in execute_command ()
#5  0x0806ce92 in execute_command_internal ()
#6  0x0806f7c3 in execute_shell_function ()
#7  0x0806d91d in execute_command_internal ()
#8  0x08070459 in execute_command ()
#9  0x0806d036 in execute_command_internal ()
#10 0x0806f9b0 in execute_command ()
#11 0x0805e996 in reader_loop ()
#12 0x0805e4a6 in main ()

And for 3.1-2 compiled locally:

#0  0x0807a741 in cleanup_dead_jobs () at ../bash/jobs.c:816
816           if (jobs[i] && DEADJOB (i) && IS_NOTIFIED (i))

#0  0x0807a741 in cleanup_dead_jobs () at ../bash/jobs.c:816
#1  0x0807acaa in wait_for (pid=23336) at ../bash/jobs.c:2395
#2  0x0806d468 in execute_command_internal (command=0x81894e8, asynchronous=0, 
    pipe_in=-1, pipe_out=-1, fds_to_close=0x8189468)
    at ../bash/execute_cmd.c:707
#3  0x0806f086 in execute_command (command=0x81894e8)
    at ../bash/execute_cmd.c:350
#4  0x0806f7bf in execute_while_or_until (while_command=0x81895a8, type=0)
    at ../bash/execute_cmd.c:2318
#5  0x0806c684 in execute_command_internal (command=0x81894c8, asynchronous=0, 
    pipe_in=-1, pipe_out=-1, fds_to_close=0x8189408)
    at ../bash/execute_cmd.c:2264
#6  0x0806ee7b in execute_in_subshell (command=0x81894c8, asynchronous=0, 
    pipe_in=Variable "pipe_in" is not available.
) at ../bash/execute_cmd.c:1311
#7  0x0806cff7 in execute_command_internal (command=0x81894c8, asynchronous=0, 
    pipe_in=3, pipe_out=-1, fds_to_close=0x8189408)
    at ../bash/execute_cmd.c:539
#8  0x0806fb4b in execute_connection (command=0x81894c8, asynchronous=0, 
    pipe_in=3, pipe_out=-1, fds_to_close=0x8189408)
    at ../bash/execute_cmd.c:1433
#9  0x0806c841 in execute_command_internal (command=0x8189448, asynchronous=0, 
    pipe_in=-1, pipe_out=-1, fds_to_close=0x8189408)
    at ../bash/execute_cmd.c:819

On Thu, Jan 12, 2006 at 04:45:53PM +1100, Laird Breyer wrote:
> Package: bash
> Version: 3.1-1
> Severity: normal
> 
> The following command segfaults at line 4097:
> 
> % seq 0 10000 | while read line; do /bin/echo $line ; done | tail -2
> 4095
> 4096
> 
> The seq, echo and tail commands are for illustration only, they are
> not related to the bug. I originally discovered the bug with completely
> different commands, but the above is the simplest illustration of the bug.
> 
> The segfault appears when the input reaches line 4097, and when an
> external command is invoked. 
> If you use the builtin echo, the bug doesn't appear:
> 
> % seq 0 10000 | while read line; do echo $line ; done | tail -2
> 9999
> 10000

Message #17 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Jan Niehusmann <jan@gondor.com>

To: 347695@bugs.debian.org

Subject: Patch to fix bug

Date: Fri, 3 Feb 2006 22:41:40 +0100

The reported behaviour is caused by an array not completely initialized
by realloc_jobs_list. The following patch fixes the bug:

--- jobs.c.orig	2006-02-03 22:27:07.000000000 +0100
+++ jobs.c	2006-02-03 22:27:58.000000000 +0100
@@ -858,6 +858,9 @@
   for (i = j = 0; i < js.j_jobslots; i++)
     if (jobs[i])
       nlist[j++] = jobs[i];
+  
+  for (; j<nsize; j++)
+    nlist[j] = 0;
 
   js.j_firstj = 0;
   js.j_lastj = (j > 0) ? j - 1: 0;

Message #22 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Justin Pryzby <justinpryzby@users.sourceforge.net>

To: Jan Niehusmann <jan@gondor.com>, 347695@bugs.debian.org

Subject: Re: Bug#347695: Patch to fix bug

Date: Fri, 3 Feb 2006 16:52:12 -0500

On Fri, Feb 03, 2006 at 10:41:40PM +0100, Jan Niehusmann wrote:
> The reported behaviour is caused by an array not completely initialized
> by realloc_jobs_list. The following patch fixes the bug:
> 
> --- jobs.c.orig	2006-02-03 22:27:07.000000000 +0100
> +++ jobs.c	2006-02-03 22:27:58.000000000 +0100
> @@ -858,6 +858,9 @@
>    for (i = j = 0; i < js.j_jobslots; i++)
>      if (jobs[i])
>        nlist[j++] = jobs[i];
> +  
> +  for (; j<nsize; j++)
> +    nlist[j] = 0;
Why not:
  memset(nlist+j, 0, (nsize-j)*sizeof(*nlist));

which also doesnn't modify j (unless you intended that j=nsize).

Justin

Message #27 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Jan Niehusmann <jan@gondor.com>

To: Justin Pryzby <justinpryzby@users.sourceforge.net>

Cc: 347695@bugs.debian.org

Subject: Re: Bug#347695: Patch to fix bug

Date: Fri, 3 Feb 2006 22:56:52 +0100

On Fri, Feb 03, 2006 at 04:52:12PM -0500, Justin Pryzby wrote:
> On Fri, Feb 03, 2006 at 10:41:40PM +0100, Jan Niehusmann wrote:

> > +  for (; j<nsize; j++)
> > +    nlist[j] = 0;
> Why not:
>   memset(nlist+j, 0, (nsize-j)*sizeof(*nlist));

Should work as well, but is harder to read (IMHO).
BTW, look at line 530:

      jobs = (JOB **)xrealloc (jobs, (js.j_jobslots * sizeof (JOB *)));

      for (j = i; j < js.j_jobslots; j++)
        jobs[j] = (JOB *)NULL;

so perhaps, in analogy to the existing code, the best version would be

  for(; j<nsize; j++)
    nlist[j] = (JOB *)NULL;

Jan

Message #34 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Jan Niehusmann <jan@gondor.com>

To: Justin Pryzby <justinpryzby@users.sourceforge.net>

Cc: 347695@bugs.debian.org

Subject: Re: Bug#347695: Patch to fix bug

Date: Fri, 3 Feb 2006 23:03:04 +0100

Oops, sorry, I missed the important point you made:

On Fri, Feb 03, 2006 at 04:52:12PM -0500, Justin Pryzby wrote:
> which also doesnn't modify j (unless you intended that j=nsize).

No, you are right, j probably should not be modified. If I understand
the code correctly, setting j (and therefore js.j_lastj) to nsize does
not really cause a problem, but is suboptimal, as it would immediately
trigger a reallocation of the jobs array.

Jan

Message #39 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Jan Niehusmann <jan@gondor.com>

To: 347695@bugs.debian.org

Cc: Justin Pryzby <justinpryzby@users.sourceforge.net>

Subject: Updated Patch

Date: Fri, 3 Feb 2006 23:13:26 +0100

On Fri, Feb 03, 2006 at 04:52:12PM -0500, Justin Pryzby wrote:
> which also doesnn't modify j (unless you intended that j=nsize).

So what about the following patch? (Already prefixed with dpatch script)

#! /bin/sh -e

if [ $# -eq 3 -a "$2" = '-d' ]; then
    pdir="-d $3"
elif [ $# -ne 1 ]; then
    echo >&2 "`basename $0`: script expects -patch|-unpatch as argument"
    exit 1
fi
case "$1" in
    -patch) patch $pdir -f --no-backup-if-mismatch -p1 < $0;;
    -unpatch) patch $pdir -f --no-backup-if-mismatch -R -p1 < $0;;
    *)
	echo >&2 "`basename $0`: script expects -patch|-unpatch as argument"
	exit 1
esac
exit 0

# DP: Fix segfault on job handling (Closes: Bug#347695)

--- bash/jobs.c.orig	2006-02-03 22:27:07.000000000 +0100
+++ bash/jobs.c	2006-02-03 22:27:58.000000000 +0100
@@ -858,6 +858,9 @@
   for (i = j = 0; i < js.j_jobslots; i++)
     if (jobs[i])
       nlist[j++] = jobs[i];
+  
+  for (i=j; i<nsize; i++)
+    nlist[i] = (JOB *)NULL;
 
   js.j_firstj = 0;
   js.j_lastj = (j > 0) ? j - 1: 0;

Message #44 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Justin Pryzby <justinpryzby@users.sourceforge.net>

To: Jan Niehusmann <jan@gondor.com>

Cc: 347695@bugs.debian.org

Subject: Re: Updated Patch

Date: Fri, 3 Feb 2006 17:15:38 -0500

On Fri, Feb 03, 2006 at 11:13:26PM +0100, Jan Niehusmann wrote:
> On Fri, Feb 03, 2006 at 04:52:12PM -0500, Justin Pryzby wrote:
> > which also doesnn't modify j (unless you intended that j=nsize).
> 
> So what about the following patch? (Already prefixed with dpatch script)
Okay, unless i=js.j_jobslots is assumed rather than i=nsize.
Justin

Message #49 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Justin Pryzby <justinpryzby@users.sourceforge.net>

To: Jan Niehusmann <jan@gondor.com>

Cc: 347695@bugs.debian.org

Subject: Re: Updated Patch

Date: Fri, 3 Feb 2006 17:33:12 -0500

On Fri, Feb 03, 2006 at 11:27:00PM +0100, Jan Niehusmann wrote:
> On Fri, Feb 03, 2006 at 05:15:38PM -0500, Justin Pryzby wrote:
> > Okay, unless i=js.j_jobslots is assumed rather than i=nsize.
> 
> Should be fine, as i is not used later in that function.
> 
> Do you happen to know what't the best way to report the fix upstream?
Probably forward to chet@po.cwru.edu, as in bash.1, and mark the bug
as such.

Thanks
Justin

Message #54 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Jan Niehusmann <jan@gondor.com>

To: Justin Pryzby <justinpryzby@users.sourceforge.net>

Cc: 347695@bugs.debian.org

Subject: Re: Updated Patch

Date: Fri, 3 Feb 2006 23:52:18 +0100

On Fri, Feb 03, 2006 at 05:33:12PM -0500, Justin Pryzby wrote:
> Probably forward to chet@po.cwru.edu, as in bash.1, and mark the bug
> as such.

I found the address bug-bash@gnu.org on the bash homepage, and used it
to report the bug with the patch.

Jan

Message #59 received at 347695@bugs.debian.org (full text, mbox, reply):

From: Jan Niehusmann <jan@gondor.com>

To: 347695@bugs.debian.org

Subject: Also fixed by upstream

Date: Sun, 12 Feb 2006 19:56:21 +0100

I got informed by Mike Frysinger that this bug has been fixed upstream. See
http://lists.nongnu.org/archive/html/bug-bash/2006-02/msg00015.html for
a patch.

Jan

Message #64 received at 347695-done@bugs.debian.org (full text, mbox, reply):

From: Matthias Klose <doko@cs.tu-berlin.de>

To: 347695-done@bugs.debian.org

Subject: Re: bash: segfault in "while read" loop

Date: Sat, 4 Mar 2006 03:32:22 +0100

Version: 3.1-3

Fixed.

Send a report that this bug log contains spam.