1506344 – Possible out of bounds write in xec.c:iousepipe

RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.

Bug 1506344 - Possible out of bounds write in xec.c:iousepipe

Summary: Possible out of bounds write in xec.c:iousepipe

Keywords:
Status:	CLOSED CURRENTRELEASE
Alias:	None
Product:	Red Hat Enterprise Linux 6
Classification:	Red Hat
Component:	ksh
Sub Component:
Version:	6.7
Hardware:	All
OS:	Linux
Priority:	urgent
Severity:	medium
Target Milestone:	rc
Target Release:	---
Assignee:	Siteshwar Vashisht
QA Contact:	BaseOS QE - Apps
Docs Contact:
URL:
Whiteboard:
Depends On:
Blocks:	1494481 1506347 1536977 1537053
TreeView+	depends on / blocked

Reported:	2017-10-25 18:47 UTC by Paulo Andrade
Modified:	2021-06-10 13:21 UTC (History)
CC List:	8 users (show)
Fixed In Version:
Doc Type:	If docs needed, set a value
Doc Text:
Clone Of:
Clones:	1506347 1536977 1537053 (view as bug list)
Environment:
Last Closed:	2018-06-21 08:47:59 UTC
Target Upstream Version:
Embargoed:

Attachments	(Terms of Use)
ksh-20120801-out-of-bounds.patch (533 bytes, patch) 2017-10-30 14:16 UTC, Paulo Andrade	kdudka: review+	Details \| Diff
Validate duplicated fd in iousepipe() (367 bytes, patch) 2017-12-21 05:21 UTC, Siteshwar Vashisht	kdudka: review+	Details \| Diff
Show Obsolete (1) View All

Description Paulo Andrade 2017-10-25 18:47:57 UTC

107: int iousepipe(Shell_t *shp)
108: {
109: 	int fd=sffileno(sfstdout),i,err=errno;
110: 	if(usepipe)
111: 	{
112: 		usepipe++;
113: 		iounpipe(shp);
114: 	}
115: 	if(sh_rpipe(subpipe) < 0)
116: 		return(0);
117: 	usepipe++;
118: 	if(shp->comsub!=1)
119: 	{
120: 		subpipe[2] = sh_fcntl(subpipe[1],F_DUPFD,10);
121: 		sh_close(subpipe[1]);
122: 		return(1);
123: 	}
124: 	subpipe[2] = sh_fcntl(fd,F_dupfd_cloexec,10);
125: 	shp->fdstatus[subpipe[2]] = shp->fdstatus[1];

in line 125 it might write out of bounds if subpipe[2] is set to
a value >= sh.gd.lim.open_max

  This is one possible reason for a crash I am looking at:

(gdb) frame 6
#6  0x0000000000424dd2 in sh_close (fd=60) at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/io.c:691
691			sh_iovalidfd(shp,fd);
(gdb) p shp.gd.lim.open_max
$1 = 32
(gdb) frame 7
#7  0x0000000000456cfe in iounpipe (shp=0x76d160) at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/xec.c:170
170		sh_close(subpipe[2]);

and it for sure did write to random memory when previously called
iousepipe.

  The crash is in a special build using glibc malloc:

#0  0x000000334fe32495 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x000000334fe33c75 in abort () at abort.c:92
#2  0x000000334fe703a7 in __libc_message (do_abort=2, fmt=0x334ff58b80 "*** glibc detected *** %s: %s: 0x%s ***\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:198
#3  0x000000334fe75dee in malloc_printerr (action=3, str=0x334ff58e80 "free(): invalid next size (normal)", 
    ptr=<value optimized out>, ar_ptr=<value optimized out>) at malloc.c:6360
#4  0x000000334fe78c80 in _int_free (av=0x335018e120, p=0x96ec20, have_lock=0) at malloc.c:4846
#5  0x00000000004241ee in sh_iovalidfd (shp=0x76d160, fd=<value optimized out>)
    at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/io.c:435
#6  0x0000000000424dd2 in sh_close (fd=60) at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/io.c:691
#7  0x0000000000456cfe in iounpipe (shp=0x76d160) at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/xec.c:170
#8  0x00000000004570f6 in _sh_fork (shp=0x76d160, parent=16219, flags=<value optimized out>, jobid=0x7fffffffba28)
...

Comment 2 Paulo Andrade 2017-10-30 13:04:54 UTC

  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}

Comment 3 Paulo Andrade 2017-10-30 13:24:55 UTC

  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

Comment 4 Paulo Andrade 2017-10-30 14:16:47 UTC

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 5 Kamil Dudka 2017-11-02 12:30:51 UTC

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.

Comment 6 Siteshwar Vashisht 2017-11-02 12:39:40 UTC

LGTM too.

Comment 13 Siteshwar Vashisht 2017-12-21 05:21:55 UTC

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

Comment 15 Kamil Dudka 2017-12-21 09:47:10 UTC

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.

Note You need to log in before you can comment on or make changes to this bug.