Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 17 Apr 2014 21:43:34 +0000 (UTC)
From:      Jilles Tjoelker <jilles@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org
Subject:   svn commit: r264629 - stable/9/bin/sh
Message-ID:  <201404172143.s3HLhYrS065535@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jilles
Date: Thu Apr 17 21:43:34 2014
New Revision: 264629
URL: http://svnweb.freebsd.org/changeset/base/264629

Log:
  MFC r263777: sh: Fix possible memory leaks and double frees with unexpected
  SIGINT.

Modified:
  stable/9/bin/sh/alias.c
  stable/9/bin/sh/exec.c
  stable/9/bin/sh/expand.c
  stable/9/bin/sh/redir.c
  stable/9/bin/sh/var.c
Directory Properties:
  stable/9/bin/sh/   (props changed)

Modified: stable/9/bin/sh/alias.c
==============================================================================
--- stable/9/bin/sh/alias.c	Thu Apr 17 21:29:22 2014	(r264628)
+++ stable/9/bin/sh/alias.c	Thu Apr 17 21:43:34 2014	(r264629)
@@ -224,6 +224,7 @@ printaliases(void)
 	int i, j;
 	struct alias **sorted, *ap;
 
+	INTOFF;
 	sorted = ckmalloc(aliases * sizeof(*sorted));
 	j = 0;
 	for (i = 0; i < ATABSIZE; i++)
@@ -231,9 +232,13 @@ printaliases(void)
 			if (*ap->name != '\0')
 				sorted[j++] = ap;
 	qsort(sorted, aliases, sizeof(*sorted), comparealiases);
-	for (i = 0; i < aliases; i++)
+	for (i = 0; i < aliases; i++) {
 		printalias(sorted[i]);
+		if (int_pending())
+			break;
+	}
 	ckfree(sorted);
+	INTON;
 }
 
 int

Modified: stable/9/bin/sh/exec.c
==============================================================================
--- stable/9/bin/sh/exec.c	Thu Apr 17 21:29:22 2014	(r264628)
+++ stable/9/bin/sh/exec.c	Thu Apr 17 21:43:34 2014	(r264629)
@@ -629,6 +629,7 @@ defun(const char *name, union node *func
 
 /*
  * Delete a function if it exists.
+ * Called with interrupts off.
  */
 
 int

Modified: stable/9/bin/sh/expand.c
==============================================================================
--- stable/9/bin/sh/expand.c	Thu Apr 17 21:29:22 2014	(r264628)
+++ stable/9/bin/sh/expand.c	Thu Apr 17 21:43:34 2014	(r264629)
@@ -986,6 +986,7 @@ recordregion(int start, int end, int inq
 {
 	struct ifsregion *ifsp;
 
+	INTOFF;
 	if (ifslastp == NULL) {
 		ifsp = &ifsfirst;
 	} else {
@@ -993,6 +994,7 @@ recordregion(int start, int end, int inq
 		    && ifslastp->inquotes == inquotes) {
 			/* extend previous area */
 			ifslastp->endoff = end;
+			INTON;
 			return;
 		}
 		ifsp = (struct ifsregion *)ckmalloc(sizeof (struct ifsregion));
@@ -1003,6 +1005,7 @@ recordregion(int start, int end, int inq
 	ifslastp->begoff = start;
 	ifslastp->endoff = end;
 	ifslastp->inquotes = inquotes;
+	INTON;
 }
 
 

Modified: stable/9/bin/sh/redir.c
==============================================================================
--- stable/9/bin/sh/redir.c	Thu Apr 17 21:29:22 2014	(r264628)
+++ stable/9/bin/sh/redir.c	Thu Apr 17 21:43:34 2014	(r264629)
@@ -93,6 +93,13 @@ static int openhere(union node *);
  * undone by calling popredir.  If the REDIR_BACKQ flag is set, then the
  * standard output, and the standard error if it becomes a duplicate of
  * stdout, is saved in memory.
+*
+ * We suppress interrupts so that we won't leave open file
+ * descriptors around.  Because the signal handler remains
+ * installed and we do not use system call restart, interrupts
+ * will still abort blocking opens such as fifos (they will fail
+ * with EINTR). There is, however, a race condition if an interrupt
+ * arrives after INTOFF and before open blocks.
  */
 
 void
@@ -104,6 +111,7 @@ redirect(union node *redir, int flags)
 	int fd;
 	char memory[10];	/* file descriptors to write to memory */
 
+	INTOFF;
 	for (i = 10 ; --i >= 0 ; )
 		memory[i] = 0;
 	memory[1] = flags & REDIR_BACKQ;
@@ -140,11 +148,14 @@ redirect(union node *redir, int flags)
 		if (fd == 0)
 			fd0_redirected++;
 		openredirect(n, memory);
+		INTON;
+		INTOFF;
 	}
 	if (memory[1])
 		out1 = &memout;
 	if (memory[2])
 		out2 = &memout;
+	INTON;
 }
 
 
@@ -157,15 +168,6 @@ openredirect(union node *redir, char mem
 	int f;
 	int e;
 
-	/*
-	 * We suppress interrupts so that we won't leave open file
-	 * descriptors around.  Because the signal handler remains
-	 * installed and we do not use system call restart, interrupts
-	 * will still abort blocking opens such as fifos (they will fail
-	 * with EINTR). There is, however, a race condition if an interrupt
-	 * arrives after INTOFF and before open blocks.
-	 */
-	INTOFF;
 	memory[fd] = 0;
 	switch (redir->nfile.type) {
 	case NFROM:
@@ -238,7 +240,6 @@ movefd:
 	default:
 		abort();
 	}
-	INTON;
 }
 
 

Modified: stable/9/bin/sh/var.c
==============================================================================
--- stable/9/bin/sh/var.c	Thu Apr 17 21:29:22 2014	(r264628)
+++ stable/9/bin/sh/var.c	Thu Apr 17 21:43:34 2014	(r264629)
@@ -263,6 +263,7 @@ setvar(const char *name, const char *val
 	} else {
 		len += strlen(val);
 	}
+	INTOFF;
 	nameeq = ckmalloc(len);
 	memcpy(nameeq, name, namelen);
 	nameeq[namelen] = '=';
@@ -271,6 +272,7 @@ setvar(const char *name, const char *val
 	else
 		nameeq[namelen + 1] = '\0';
 	setvareq(nameeq, flags);
+	INTON;
 }
 
 static int
@@ -303,6 +305,7 @@ change_env(const char *s, int set)
 	char *eqp;
 	char *ss;
 
+	INTOFF;
 	ss = savestr(s);
 	if ((eqp = strchr(ss, '=')) != NULL)
 		*eqp = '\0';
@@ -311,6 +314,7 @@ change_env(const char *s, int set)
 	else
 		(void) unsetenv(ss);
 	ckfree(ss);
+	INTON;
 
 	return;
 }
@@ -373,13 +377,13 @@ setvareq(char *s, int flags)
 	/* not found */
 	if (flags & VNOSET)
 		return;
+	INTOFF;
 	vp = ckmalloc(sizeof (*vp));
 	vp->flags = flags;
 	vp->text = s;
 	vp->name_len = nlen;
 	vp->next = *vpp;
 	vp->func = NULL;
-	INTOFF;
 	*vpp = vp;
 	if ((vp->flags & VEXPORT) && localevar(s)) {
 		change_env(s, 1);
@@ -792,6 +796,7 @@ poplocalvars(void)
 	struct localvar *lvp;
 	struct var *vp;
 
+	INTOFF;
 	while ((lvp = localvars) != NULL) {
 		localvars = lvp->next;
 		vp = lvp->vp;
@@ -809,6 +814,7 @@ poplocalvars(void)
 		}
 		ckfree(lvp);
 	}
+	INTON;
 }
 
 
@@ -847,18 +853,21 @@ unsetcmd(int argc __unused, char **argv 
 	if (flg_func == 0 && flg_var == 0)
 		flg_var = 1;
 
+	INTOFF;
 	for (ap = argptr; *ap ; ap++) {
 		if (flg_func)
 			ret |= unsetfunc(*ap);
 		if (flg_var)
 			ret |= unsetvar(*ap);
 	}
+	INTON;
 	return ret;
 }
 
 
 /*
  * Unset the specified variable.
+ * Called with interrupts off.
  */
 
 int
@@ -872,7 +881,6 @@ unsetvar(const char *s)
 		return (0);
 	if (vp->flags & VREADONLY)
 		return (1);
-	INTOFF;
 	if (vp->text[vp->name_len + 1] != '\0')
 		setvar(s, nullstr, 0);
 	if ((vp->flags & VEXPORT) && localevar(vp->text)) {
@@ -888,7 +896,6 @@ unsetvar(const char *s)
 		*vpp = vp->next;
 		ckfree(vp);
 	}
-	INTON;
 	return (0);
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201404172143.s3HLhYrS065535>