Bug 1506344

  The pattern being found is 0xa 'IOSEEK|IOWRITE'.
  In a previous crash, this was found when it crashed in the free call in
sh_iovalidfd:

"""
	Sfio_t		**sftable = shp->sftable;
...
	max = shp->gd->lim.open_max;
	shp->sftable = (Sfio_t**)calloc((n+1)*(sizeof(int*)+sizeof(Sfio_t*)+1),1);
	if(max)
		memcpy(shp->sftable,sftable,max*sizeof(Sfio_t*));
...
	if(sftable)
		free((void*)sftable);

and the crash is in the free call, but the addresses do not look like
related in a way that could corrupt glibc malloc metadata.

  The out of bounds write was at in iousepipe():
   shp->fdstatus[subpipe[2]] = shp->fdstatus[1];

(gdb) p sh.gd.lim
$1 = {arg_max = 2621440, open_max = 32, clk_tck = 100, child_max = 16384, ngroups_max = 65536, posix_version = 188 '\274', 
  posix_jobcontrol = 1 '\001', fs3d = 0 '\000'}

  Using the previous value of sh.sftable (the sftable variable being released), and
knowing how the data is packed:

 (gdb) p ((char*)(sftable+64))[1]
$2 = 10 '\n'

and this memory would be 28 bytes after the end of the buffer:
(gdb) p ((char*)(sftable+64))[60]
$3 = 10 '\n'

  I said the addresses do not look related because:

(gdb) p sftable
$4 = (Sfio_t **) 0x96ec30
(gdb) p sh.sftable
$5 = (Sfio_t **) 0x9b66e0
(gdb) p 0x9b66e0-0x96ec30
$6 = 293552
(gdb) p 293552/17
$7 = 17267

17 is the size of the packed data, and in that space it would fit 17267
entries, so the pointers look quite distant in memory.
"""

  On a new crash, 0xa appears again:
Program terminated with signal 11, Segmentation fault.
#0  sfraise (f=0x76ada0, type=14, data=0x0)
    at /usr/src/debug/ksh-20120801/src/lib/libast/sfio/sfraise.c:84
84		{	next = disc->disc;
(gdb) bt
#0  sfraise (f=0x76ada0, type=14, data=0x0)
    at /usr/src/debug/ksh-20120801/src/lib/libast/sfio/sfraise.c:84
#1  0x00000000004c4304 in _sfcleanup ()
    at /usr/src/debug/ksh-20120801/src/lib/libast/sfio/sfmode.c:91
#2  0x000000334fe35992 in __run_exit_handlers (status=0) at exit.c:78
#3  exit (status=0) at exit.c:100
#4  0x000000000041aa35 in sh_done (ptr=0x76d160, sig=0)
    at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/fault.c:671
#5  0x0000000000407dac in sh_main (ac=<value optimized out>, 
    av=0x7fffffffcb38, userinit=<value optimized out>)
    at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/main.c:354
#6  0x000000334fe1ed1d in __libc_start_main (main=0x406ae0 <main>, argc=4, 
    ubp_av=0x7fffffffcb38, init=<value optimized out>, 
    fini=<value optimized out>, rtld_fini=<value optimized out>, 
    stack_end=0x7fffffffcb28) at libc-start.c:226
#7  0x0000000000406a19 in _start ()
(gdb) p disc
$1 = (Sfdisc_t *) 0xa000000000000

  But the last coredump has plently of other paths to look. This is again a
build using glibc malloc, but also MALLOC_PERTURB_=90 to help in detecting
use after free, and it indeed happens:

(gdb) p *f
$2 = {
  next = 0x96efc0 "\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245"..., 
  endw = 0x96efc0 "\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245"..., 
---Type <return> to continue, or q <return> to quit---q
endr = 0x96efc0 "\245\245\245\245\245\245\245\245\245\2Quit
(gdb) p/o ~90&0xff
$3 = 0245

So, it is using released stream buffers. Might be some bad realloc logic
somewhere (like a save/restore state not accounting a buffer realloc):

\245"..., size = 65536, val = 65536, extent = -1, here = 0, 
  unused_1 = 0 '\000', tiny = "", bits = 0, mode = 33, disc = 0x96ee70, 
  pool = 0x76afe8, rsrv = 0x0, proc = 0x0, mutex = 0x0, stdio = 0x0, lpos = 0, 
  iosz = 0, blksz = 1024, getr = 0}
(gdb) p *f.disc
$4 = {readf = 0x423990 <piperead>, writef = 0, seekf = 0, 
  exceptf = 0x423ed0 <slowexcept>, disc = 0xa000000000000}

  Since the crash is in an atexit handler, the \245\245 buffers should
be during data release, and/or a side effect of data corruption.

  On another crash there is this pattern:

#0  0x00000000000a3ed0 in ?? ()
(gdb) bt
#0  0x00000000000a3ed0 in ?? ()
#1  0x00000000004ca14f in sfsync (f=0x76ada0)
...
(gdb) frame 1
#1  0x00000000004ca14f in sfsync (f=0x76ada0)
    at /usr/src/debug/ksh-20120801/src/lib/libast/sfio/sfsync.c:117
117				(void)(*f->disc->exceptf)(f,SF_SYNC,(Void_t*)((int)1),f->disc);
(gdb) p f.disc
$1 = (struct _sfdisc_s *) 0x96ee70
(gdb) p* f.disc
$2 = {readf = 0x423990 <piperead>, writef = 0, seekf = 0, exceptf = 0xa3ed0, 
  disc = 0x0}
(gdb) x/i 0x423ed0
   0x423ed0 <slowexcept>:	mov    %rbx,-0x20(%rsp)

  Note again the 0xa, that replaced by 0x42 (following the pattern of the readf
callback) turns out to be the pointer it should have had:
 0xa3ed0
0x423ed0

Created attachment 1345451 [details]
ksh-20120801-out-of-bounds.patch

  This patch corrects this specific problem.

  Note also that it is kind of a minimal patch for the issue in
bug #1259898, that, by treating F_dupfd_cloexec specially, also
calls sh_iovalidfd(shp,newfd); if newfd returned by fcntl(F_DUPFD)
would have an offset out of bounds in the shell data structures.

Comment on attachment 1345451 [details]
ksh-20120801-out-of-bounds.patch

Looks good to me.  Btw. bug #1463312 (out of bound access on the same array at index -1) seems to be also related.

LGTM too.

Created attachment 1370722 [details]
Validate duplicated fd in iousepipe()

Comment on attachment 1370722 [details]
Validate duplicated fd in iousepipe()

The condition in attachment #1345451 [details] was in fact unnecessary but harmless.  This patch should be functionally equivalent as I understand it.