Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/bin/sh Improve the solution for the 2nd access to a fd which...
details: https://anonhg.NetBSD.org/src/rev/bd1b44731eba
branches: trunk
changeset: 986257:bd1b44731eba
user: kre <kre%NetBSD.org@localhost>
date: Wed Sep 15 18:29:45 2021 +0000
description:
Improve the solution for the 2nd access to a fd which shouldn't
be available ("13") issue reported by Jan Schaumann on netbsd-users.
This fixes a bug in the earlier fix (a day or so ago) which could allow the
shell's idea of which fd range was in use by the script to get wildly
incorrect, but now also actually looks to see which fd's are in use as
renamed other user fd's during the lifetime of a redirection which needs
to be able to be undone (most redirections occur after a fork and are
permanent in the child process). Attempting to access such a fd (as with
attempts to access a fd in use by the shell for its own purposes) is treated
as an attempt to access a closed fd (EBADF). Attempting to reuse the fd
for some other purpose (which should be rare, even for scripts attempting
to cause problems, since the shell generally knows which fds the script
wants to use, and avoids them) will cause the renamed (renumbered) fd
to be renamed again (moved aside to some other available fd), just as
happens with the shell's private fds.
Also, when a generic fd is required, don't give up because of EMFILE
or similar unless there are no available fds at all (we might prefer >10
or bigger, but if there are none there, use anything). This avoids
redirection errors when ulimit -n has been set small, and all the fds >10
that are available have been used, but we need somewhere to park the old
user of a fd while we reuse that fd for the redirection.
diffstat:
bin/sh/main.c | 6 ++-
bin/sh/parser.c | 18 ++++++-----
bin/sh/redir.c | 91 +++++++++++++++++++++++++++++++++++++++++++-------------
bin/sh/redir.h | 3 +-
4 files changed, 85 insertions(+), 33 deletions(-)
diffs (297 lines):
diff -r 041fa27661eb -r bd1b44731eba bin/sh/main.c
--- a/bin/sh/main.c Wed Sep 15 17:33:08 2021 +0000
+++ b/bin/sh/main.c Wed Sep 15 18:29:45 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: main.c,v 1.85 2020/02/07 01:25:08 fox Exp $ */
+/* $NetBSD: main.c,v 1.86 2021/09/15 18:29:45 kre Exp $ */
/*-
* Copyright (c) 1991, 1993
@@ -42,7 +42,7 @@
#if 0
static char sccsid[] = "@(#)main.c 8.7 (Berkeley) 7/19/95";
#else
-__RCSID("$NetBSD: main.c,v 1.85 2020/02/07 01:25:08 fox Exp $");
+__RCSID("$NetBSD: main.c,v 1.86 2021/09/15 18:29:45 kre Exp $");
#endif
#endif /* not lint */
@@ -85,6 +85,7 @@
int rootshell;
struct jmploc main_handler;
int max_user_fd;
+long user_fd_limit;
#if PROFILE
short profile_buf[16384];
extern int etext();
@@ -131,6 +132,7 @@
max_user_fd = fcntl(0, F_MAXFD);
if (max_user_fd < 2)
max_user_fd = 2;
+ user_fd_limit = sysconf(_SC_OPEN_MAX);
setlocale(LC_ALL, "");
diff -r 041fa27661eb -r bd1b44731eba bin/sh/parser.c
--- a/bin/sh/parser.c Wed Sep 15 17:33:08 2021 +0000
+++ b/bin/sh/parser.c Wed Sep 15 18:29:45 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: parser.c,v 1.173 2021/09/14 14:49:39 kre Exp $ */
+/* $NetBSD: parser.c,v 1.174 2021/09/15 18:29:45 kre Exp $ */
/*-
* Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
#if 0
static char sccsid[] = "@(#)parser.c 8.7 (Berkeley) 5/16/95";
#else
-__RCSID("$NetBSD: parser.c,v 1.173 2021/09/14 14:49:39 kre Exp $");
+__RCSID("$NetBSD: parser.c,v 1.174 2021/09/15 18:29:45 kre Exp $");
#endif
#endif /* not lint */
@@ -746,9 +746,12 @@
if (!err)
n->ndup.vname = NULL;
- if (is_number(text))
+ if (is_number(text)) {
n->ndup.dupfd = number(text);
- else if (text[0] == '-' && text[1] == '\0')
+ if (n->ndup.dupfd < user_fd_limit &&
+ n->ndup.dupfd > max_user_fd)
+ max_user_fd = n->ndup.dupfd;
+ } else if (text[0] == '-' && text[1] == '\0')
n->ndup.dupfd = -1;
else {
@@ -757,8 +760,6 @@
else
n->ndup.vname = makeword(startlinno - elided_nl);
}
- if (n->ndup.dupfd > max_user_fd)
- max_user_fd = n->ndup.dupfd;
}
@@ -1592,8 +1593,9 @@
fd = (*out == '\0') ? -1 : number(out); /* number(out) >= 0 */
np->nfile.fd = fd; /* do this again later with updated fd */
if (fd != np->nfile.fd)
- error("file descriptor (%d) out of range", fd);
- if (fd > max_user_fd)
+ error("file descriptor (%d) out of range (max %ld)",
+ fd, user_fd_limit - 1);
+ if (fd < user_fd_limit && fd > max_user_fd)
max_user_fd = fd;
VTRACE(DBG_LEXER, ("parseredir after '%s%c' ", out, c));
diff -r 041fa27661eb -r bd1b44731eba bin/sh/redir.c
--- a/bin/sh/redir.c Wed Sep 15 17:33:08 2021 +0000
+++ b/bin/sh/redir.c Wed Sep 15 18:29:45 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: redir.c,v 1.67 2021/09/14 14:49:39 kre Exp $ */
+/* $NetBSD: redir.c,v 1.68 2021/09/15 18:29:45 kre Exp $ */
/*-
* Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
#if 0
static char sccsid[] = "@(#)redir.c 8.2 (Berkeley) 5/4/95";
#else
-__RCSID("$NetBSD: redir.c,v 1.67 2021/09/14 14:49:39 kre Exp $");
+__RCSID("$NetBSD: redir.c,v 1.68 2021/09/15 18:29:45 kre Exp $");
#endif
#endif /* not lint */
@@ -121,6 +121,7 @@
STATIC const struct renamelist *is_renamed(const struct renamelist *, int);
STATIC void fd_rename(struct redirtab *, int, int);
+STATIC int * saved_redirected_fd(int);
STATIC void free_rl(struct redirtab *, int);
STATIC void openredirect(union node *, char[10], int);
STATIC int openhere(const union node *);
@@ -136,6 +137,7 @@
STATIC struct shell_fds *sh_fd_list;
+STATIC int pick_new_fd(int);
STATIC void renumber_sh_fd(struct shell_fds *);
STATIC struct shell_fds *sh_fd(int);
@@ -150,6 +152,21 @@
return NULL;
}
+STATIC int *
+saved_redirected_fd(int fd)
+{
+ struct redirtab *rt;
+ struct renamelist *rl;
+
+ for (rt = redirlist; rt != NULL; rt = rt->next) {
+ for (rl = rt->renamed; rl != NULL; rl = rl->next) {
+ if (rl->into == fd)
+ return &rl->into;
+ }
+ }
+ return NULL;
+}
+
STATIC void
free_rl(struct redirtab *rt, int reset)
{
@@ -221,10 +238,22 @@
redirlist = sv;
}
for (n = redir ; n ; n = n->nfile.next) {
+ int *renamed;
+
fd = n->nfile.fd;
- VTRACE(DBG_REDIR, ("redir %d (max=%d) ", fd, max_user_fd));
- if (fd > max_user_fd)
+ VTRACE(DBG_REDIR, ("redir %d (max=%d limit=%ld) ",
+ fd, max_user_fd, user_fd_limit));
+ if (fd < user_fd_limit && fd > max_user_fd)
max_user_fd = fd;
+ if ((renamed = saved_redirected_fd(fd)) != NULL) {
+ int to = pick_new_fd(fd);
+
+ VTRACE(DBG_REDIR,
+ ("redirect: moved holding fd %d to %d\n", fd, to));
+ *renamed = to;
+ if (to != fd) /* always... */
+ (void)close(fd);
+ }
renumber_sh_fd(sh_fd(fd));
if ((n->nfile.type == NTOFD || n->nfile.type == NFROMFD) &&
n->ndup.dupfd == fd) {
@@ -255,6 +284,10 @@
i = fcntl(fd, F_DUPFD, big_sh_fd);
if (i >= 0)
break;
+ if (errno == EMFILE || errno == EINVAL)
+ i = fcntl(fd, F_DUPFD, 3);
+ if (i >= 0)
+ break;
/* FALLTHRU */
default:
error("%d: %s", fd, strerror(errno));
@@ -264,7 +297,7 @@
if (i >= 0)
(void)fcntl(i, F_SETFD, FD_CLOEXEC);
fd_rename(sv, fd, i);
- VTRACE(DBG_REDIR, ("saved as %d ", i));
+ VTRACE(DBG_REDIR, ("fd %d saved as %d ", fd, i));
INTON;
}
VTRACE(DBG_REDIR, ("%s\n", fd == 0 ? "STDIN" : ""));
@@ -353,7 +386,8 @@
case NTOFD:
case NFROMFD:
if (redir->ndup.dupfd >= 0) { /* if not ">&-" */
- if (sh_fd(redir->ndup.dupfd) != NULL)
+ if (sh_fd(redir->ndup.dupfd) != NULL ||
+ saved_redirected_fd(redir->ndup.dupfd) != NULL)
error("Redirect (from %d to %d) failed: %s",
redir->ndup.dupfd, fd, strerror(EBADF));
if (fd < 10 && redir->ndup.dupfd < 10 &&
@@ -604,7 +638,7 @@
int i;
VTRACE(DBG_REDIR|DBG_OUTPUT, ("to_upper_fd(%d)", fd));
- if (big_sh_fd < 10)
+ if (big_sh_fd < 10 || big_sh_fd >= user_fd_limit)
find_big_fd();
do {
i = fcntl(fd, F_DUPFD_CLOEXEC, big_sh_fd);
@@ -672,6 +706,26 @@
return NULL;
}
+STATIC int
+pick_new_fd(int fd)
+{
+ int to;
+
+ to = fcntl(fd, F_DUPFD_CLOEXEC, big_sh_fd);
+ if (to == -1 && big_sh_fd >= 22)
+ to = fcntl(fd, F_DUPFD_CLOEXEC, big_sh_fd/2);
+ if (to == -1)
+ to = fcntl(fd, F_DUPFD_CLOEXEC, fd + 1);
+ if (to == -1)
+ to = fcntl(fd, F_DUPFD_CLOEXEC, 10);
+ if (to == -1)
+ to = fcntl(fd, F_DUPFD_CLOEXEC, 3);
+ if (to == -1)
+ error("insufficient file descriptors available");
+ CLOEXEC(to);
+ return to;
+}
+
STATIC void
renumber_sh_fd(struct shell_fds *fp)
{
@@ -689,22 +743,14 @@
if (fp->fd >= big_sh_fd)
find_big_fd();
- to = fcntl(fp->fd, F_DUPFD_CLOEXEC, big_sh_fd);
- if (to == -1 && big_sh_fd >= 22)
- to = fcntl(fp->fd, F_DUPFD_CLOEXEC, big_sh_fd/2);
- if (to == -1)
- to = fcntl(fp->fd, F_DUPFD_CLOEXEC, fp->fd + 1);
- if (to == -1)
- to = fcntl(fp->fd, F_DUPFD_CLOEXEC, 10);
- if (to == -1)
- to = fcntl(fp->fd, F_DUPFD_CLOEXEC, 3);
- if (to == -1)
- error("insufficient file descriptors available");
- CLOEXEC(to);
+ to = pick_new_fd(fp->fd);
if (fp->fd == to) /* impossible? */
return;
+ VTRACE(DBG_REDIR, ("renumber_sh_fd: moved shell fd %d to %d\n",
+ fp->fd, to));
+
(*fp->cb)(fp->fd, to);
(void)close(fp->fd);
fp->fd = to;
@@ -831,7 +877,7 @@
{
int c, f;
- if (sh_fd(fd) != NULL) {
+ if (sh_fd(fd) != NULL || saved_redirected_fd(fd) != NULL) {
if (!p)
return -1;
error("Can't get status for fd=%d (%s)", fd, strerror(EBADF));
@@ -991,8 +1037,9 @@
while (num[0] == '0' && num[1] != '\0') /* skip 0's */
num++;
- if (strlen(num) > 5)
- error("%s too big to be a file descriptor", num);
+ if (strlen(num) > 5 ||
+ (fd >= user_fd_limit && fd > max_user_fd))
+ error("%s: too big to be a file descriptor", num);
if (setflags)
setone(fd, pos, neg, verbose);
diff -r 041fa27661eb -r bd1b44731eba bin/sh/redir.h
--- a/bin/sh/redir.h Wed Sep 15 17:33:08 2021 +0000
+++ b/bin/sh/redir.h Wed Sep 15 18:29:45 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: redir.h,v 1.25 2020/04/03 16:22:23 joerg Exp $ */
+/* $NetBSD: redir.h,v 1.26 2021/09/15 18:29:45 kre Exp $ */
/*-
* Copyright (c) 1991, 1993
@@ -53,3 +53,4 @@
int outredir(struct output *, union node *, int);
extern int max_user_fd; /* highest fd used by user */
+extern long user_fd_limit; /* highest possible user fd */
Home |
Main Index |
Thread Index |
Old Index