dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] redir: Retry open64 on EINTR
@ 2020-05-28 11:31 Herbert Xu
  0 siblings, 0 replies; only message in thread
From: Herbert Xu @ 2020-05-28 11:31 UTC (permalink / raw)
  To: dash, 755446, Samuel Thibault, Jilles Tjoelker

It is possible for open64 to block on named pipes, and therefore
it can be interrupted by signals and return EINTR.  We should only
let it fail with EINTR if real signals are pending (i.e., it should
not fail on SIGCHLD if SIGCHLD has not been trapped).

This patch adds a new helper sh_open to retry the open64 call if
necessary.  It also calls sh_error when appropriate.

Fixes: 3800d4934391 ("[JOBS] Fix dowait signal race")
Reported-by: Samuel Thibault <sthibault@debian.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/src/input.c b/src/input.c
index 17544e7..6103312 100644
--- a/src/input.c
+++ b/src/input.c
@@ -393,12 +393,9 @@ setinputfile(const char *fname, int flags)
 	int fd;
 
 	INTOFF;
-	if ((fd = open64(fname, O_RDONLY)) < 0) {
-		if (flags & INPUT_NOFILE_OK)
-			goto out;
-		exitstatus = 127;
-		exerror(EXERROR, "Can't open %s", fname);
-	}
+	fd = sh_open(fname, O_RDONLY, flags & INPUT_NOFILE_OK);
+	if (fd < 0)
+		goto out;
 	if (fd < 10)
 		fd = savefd(fd, fd);
 	setinputfd(fd, flags & INPUT_PUSH_FILE);
diff --git a/src/jobs.c b/src/jobs.c
index f65435d..95e7ec4 100644
--- a/src/jobs.c
+++ b/src/jobs.c
@@ -196,7 +196,7 @@ setjobctl(int on)
 		return;
 	if (on) {
 		int ofd;
-		ofd = fd = open64(_PATH_TTY, O_RDWR);
+		ofd = fd = sh_open(_PATH_TTY, O_RDWR, 1);
 		if (fd < 0) {
 			fd += 3;
 			while (!isatty(fd))
@@ -887,8 +887,7 @@ static void forkchild(struct job *jp, union node *n, int mode)
 		ignoresig(SIGQUIT);
 		if (jp->nprocs == 0) {
 			close(0);
-			if (open64(_PATH_DEVNULL, O_RDONLY) != 0)
-				sh_error("Can't open %s", _PATH_DEVNULL);
+			sh_open(_PATH_DEVNULL, O_RDONLY, 0);
 		}
 	}
 	if (!oldlvl && iflag) {
diff --git a/src/redir.c b/src/redir.c
index 895140c..93abba3 100644
--- a/src/redir.c
+++ b/src/redir.c
@@ -55,6 +55,7 @@
 #include "output.h"
 #include "memalloc.h"
 #include "error.h"
+#include "trap.h"
 
 
 #define EMPTY -2		/* marks an unused slot in redirtab */
@@ -180,56 +181,83 @@ redirect(union node *redir, int flags)
 }
 
 
+static int sh_open_fail(const char *, int, int) __attribute__((__noreturn__));
+static int sh_open_fail(const char *pathname, int flags, int e)
+{
+	const char *word;
+	int action;
+
+	word = "open";
+	action = E_OPEN;
+	if (flags & O_CREAT) {
+		word = "create";
+		action = E_CREAT;
+	}
+
+	sh_error("cannot %s %s: %s", word, pathname, errmsg(e, action));
+}
+
+
+int sh_open(const char *pathname, int flags, int mayfail)
+{
+	int fd;
+	int e;
+
+	do {
+		fd = open64(pathname, flags, 0666);
+		e = errno;
+	} while (fd < 0 && e == EINTR && !pending_sig);
+
+	if (mayfail || fd >= 0)
+		return fd;
+
+	sh_open_fail(pathname, flags, e);
+}
+
+
 STATIC int
 openredirect(union node *redir)
 {
 	struct stat64 sb;
 	char *fname;
+	int flags;
 	int f;
 
 	switch (redir->nfile.type) {
 	case NFROM:
-		fname = redir->nfile.expfname;
-		if ((f = open64(fname, O_RDONLY)) < 0)
-			goto eopen;
+		flags = O_RDONLY;
+do_open:
+		f = sh_open(redir->nfile.expfname, flags, 0);
 		break;
 	case NFROMTO:
-		fname = redir->nfile.expfname;
-		if ((f = open64(fname, O_RDWR|O_CREAT, 0666)) < 0)
-			goto ecreate;
-		break;
+		flags = O_RDWR|O_CREAT;
+		goto do_open;
 	case NTO:
 		/* Take care of noclobber mode. */
 		if (Cflag) {
 			fname = redir->nfile.expfname;
 			if (stat64(fname, &sb) < 0) {
-				if ((f = open64(fname, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0)
-					goto ecreate;
-			} else if (!S_ISREG(sb.st_mode)) {
-				if ((f = open64(fname, O_WRONLY, 0666)) < 0)
-					goto ecreate;
-				if (!fstat64(f, &sb) && S_ISREG(sb.st_mode)) {
-					close(f);
-					errno = EEXIST;
-					goto ecreate;
-				}
-			} else {
-				errno = EEXIST;
+				flags = O_WRONLY|O_CREAT|O_EXCL;
+				goto do_open;
+			}
+
+			if (S_ISREG(sb.st_mode))
+				goto ecreate;
+
+			f = sh_open(fname, O_WRONLY, 0);
+			if (!fstat64(f, &sb) && S_ISREG(sb.st_mode)) {
+				close(f);
 				goto ecreate;
 			}
 			break;
 		}
 		/* FALLTHROUGH */
 	case NCLOBBER:
-		fname = redir->nfile.expfname;
-		if ((f = open64(fname, O_WRONLY|O_CREAT|O_TRUNC, 0666)) < 0)
-			goto ecreate;
-		break;
+		flags = O_WRONLY|O_CREAT|O_TRUNC;
+		goto do_open;
 	case NAPPEND:
-		fname = redir->nfile.expfname;
-		if ((f = open64(fname, O_WRONLY|O_CREAT|O_APPEND, 0666)) < 0)
-			goto ecreate;
-		break;
+		flags = O_WRONLY|O_CREAT|O_APPEND;
+		goto do_open;
 	case NTOFD:
 	case NFROMFD:
 		f = redir->ndup.dupfd;
@@ -249,9 +277,7 @@ openredirect(union node *redir)
 
 	return f;
 ecreate:
-	sh_error("cannot create %s: %s", fname, errmsg(errno, E_CREAT));
-eopen:
-	sh_error("cannot open %s: %s", fname, errmsg(errno, E_OPEN));
+	sh_open_fail(fname, O_CREAT, EEXIST);
 }
 
 
diff --git a/src/redir.h b/src/redir.h
index 1cf2761..16f5c20 100644
--- a/src/redir.h
+++ b/src/redir.h
@@ -49,4 +49,5 @@ int savefd(int, int);
 int redirectsafe(union node *, int);
 void unwindredir(struct redirtab *stop);
 struct redirtab *pushredir(union node *redir);
+int sh_open(const char *pathname, int flags, int mayfail);
 
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-05-28 11:31 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-28 11:31 [PATCH] redir: Retry open64 on EINTR Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).