* fdtable bug
@ 1996-05-08 23:50 Zefram
1996-05-10 9:38 ` Zefram
0 siblings, 1 reply; 2+ messages in thread
From: Zefram @ 1996-05-08 23:50 UTC (permalink / raw)
To: Z Shell workers mailing list
Someone posted a bug report, which had a pointer error deep within
exec.c, and mentioned that the bug went away when the shell was run
under gdb. Well, I found a bug with exactly these properties.
Unfortunately it caused zsh to fall over while processing .zlogin, in
the account where I'd installed it. Fortunately I was able to get into
the account without root intervention.
The effect of the bug is that movefd() can occasionally write 1 into
fdtable[-1]. In my case this was the low-order byte of shfunctab, and
so caused a bus error on the next command.
This happens because movefd() may be called upon to move a file
descriptor that is not actually valid. We need to add a test in
movefd(), so that if the fd is invalid it doesn't write to fdtable.
movefd() is called from addfd() when creating a new multio, for the
purpose of saving the old file descriptor. Unfortunately, I don't see
a simple way of saving an invalid file descriptor, because -1 in the
save array means that the fd hasn't been saved yet. Maybe -2 should be
used for that, and then movefd() can return -1 for an invalid fd.
I don't include a patch, because I haven't thought through everything
involved yet. I'll probably have a patch tomorrow (er, later today,
local British time), if nobody has done it by then.
-zefram
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: fdtable bug
1996-05-08 23:50 fdtable bug Zefram
@ 1996-05-10 9:38 ` Zefram
0 siblings, 0 replies; 2+ messages in thread
From: Zefram @ 1996-05-10 9:38 UTC (permalink / raw)
To: Z Shell workers mailing list
-----BEGIN PGP SIGNED MESSAGE-----
There are actually *two* remaining bugs in redirection, after
the patch I sent two days ago. I have found and fixed them both.
I'm reasonably confident that they are the last significant bugs
in this area. We shall see.
One of the bugs, with fdtable, I've already described. That's fixed
in this patch by (1) a revision of the fd manipulation functions in
utils.c (their semantics are now documented in comments), and (2)
a change of the way the save array is used -- -2 now means `not
redirected', and -1 means `was previously closed'. This latter
change alone should fix some bugs I don't actually have test cases
for -- previously the two conditions were not properly distingushed.
The other bug is an unusual case in addfd(), if its two fd arguments
are equal. See the new comment above the function. To demonstrate:
% exec 3>&-
% echo foo >&3
zsh: 3: bad file number
% # that is as it should be
% echo foo 3> file
% echo foo >&3
% cat file
foo
% # note that fd 3 is now permanently redirected
-zefram
Index: Src/exec.c
*** exec.c 1996/05/09 21:36:14 1.5
--- exec.c 1996/05/09 23:55:43
***************
*** 978,984 ****
* two fds, the result of open("bar",...), and the result of *
* open("ble",....). */
! /* add a fd to an multio */
/**/
void
--- 978,990 ----
* two fds, the result of open("bar",...), and the result of *
* open("ble",....). */
! /* Add a fd to an multio. fd1 must be < 10, and may be in any state. *
! * fd2 must be open, and is `consumed' by this function. Note that *
! * fd1 == fd2 is possible, and indicates that fd1 was really closed. *
! * We effectively do `fd2 = movefd(fd2)' at the beginning of this *
! * function, but in most cases we can avoid an extra dup by delaying *
! * the movefd: we only >need< to move it if we're actually doing a *
! * multiple redirection. */
/**/
void
***************
*** 989,996 ****
if (!mfds[fd1] || isset(NOMULTIOS)) {
if(!mfds[fd1]) { /* starting a new multio */
mfds[fd1] = (struct multio *) alloc(sizeof(struct multio));
! if (!forked && fd1 != fd2 && fd1 < 10)
! save[fd1] = movefd(fd1);
}
redup(fd2, fd1);
mfds[fd1]->ct = 1;
--- 995,1002 ----
if (!mfds[fd1] || isset(NOMULTIOS)) {
if(!mfds[fd1]) { /* starting a new multio */
mfds[fd1] = (struct multio *) alloc(sizeof(struct multio));
! if (!forked && save[fd1] == -2)
! save[fd1] = (fd1 == fd2) ? -1 : movefd(fd1);
}
redup(fd2, fd1);
mfds[fd1]->ct = 1;
***************
*** 1110,1116 ****
type = cmd->type;
for (i = 0; i < 10; i++) {
! save[i] = -1;
mfds[i] = NULL;
}
--- 1116,1122 ----
type = cmd->type;
for (i = 0; i < 10; i++) {
! save[i] = -2;
mfds[i] = NULL;
}
***************
*** 1428,1434 ****
init_io();
break;
case CLOSE:
! if (!forked && fn->fd1 < 10 && save[fn->fd1] == -1)
save[fn->fd1] = movefd(fn->fd1);
closemn(mfds, fn->fd1);
zclose(fn->fd1);
--- 1434,1440 ----
init_io();
break;
case CLOSE:
! if (!forked && fn->fd1 < 10 && save[fn->fd1] == -2)
save[fn->fd1] = movefd(fn->fd1);
closemn(mfds, fn->fd1);
zclose(fn->fd1);
***************
*** 1477,1483 ****
if (nullexec) {
for (i = 0; i < 10; i++)
! if (save[i] != -1)
zclose(save[i]);
return;
}
--- 1483,1489 ----
if (nullexec) {
for (i = 0; i < 10; i++)
! if (save[i] != -2)
zclose(save[i]);
return;
}
***************
*** 1543,1549 ****
fprintf(stderr, "zsh: exit %ld\n", (long)lastval);
}
fflush(stdout);
! if (save[1] == -1) {
if (ferror(stdout)) {
zerr("write error: %e", NULL, errno);
clearerr(stdout);
--- 1549,1555 ----
fprintf(stderr, "zsh: exit %ld\n", (long)lastval);
}
fflush(stdout);
! if (save[1] == -2) {
if (ferror(stdout)) {
zerr("write error: %e", NULL, errno);
clearerr(stdout);
***************
*** 1673,1679 ****
int i;
for (i = 0; i != 10; i++)
! if (save[i] != -1)
redup(save[i], i);
errno = old_errno;
}
--- 1679,1685 ----
int i;
for (i = 0; i != 10; i++)
! if (save[i] != -2)
redup(save[i], i);
errno = old_errno;
}
Index: Src/utils.c
*** utils.c 1996/05/07 22:55:46 1.3
--- utils.c 1996/05/09 22:41:49
***************
*** 851,902 ****
#endif /* TIOCGWINSZ */
}
! /* move a fd to a place >= 10 and mark the new fd in fdtable. */
/**/
int
movefd(int fd)
{
! int fe;
!
! if (fd == -1 || fd >= 10)
! return fd;
#ifdef F_DUPFD
! fe = fcntl(fd, F_DUPFD, 10);
#else
! fe = movefd(dup(fd));
#endif
! close(fd);
! fdtable[fd] = 0;
! fdtable[fe] = 1;
! return fe;
}
! /* move fd x to y */
/**/
void
redup(int x, int y)
{
! if (x != y && x >= 0) {
dup2(x, y);
close(x);
fdtable[x] = 0;
- fdtable[y] = 1;
}
}
! /* close the given fd, and clear it from fdtable */
/**/
int
zclose(int fd)
{
! if (fd >= 0) {
fdtable[fd] = 0;
! return close(fd);
! }
! return EBADF;
}
/* Get a file name relative to $TMPPREFIX which *
--- 851,904 ----
#endif /* TIOCGWINSZ */
}
! /* Move a fd to a place >= 10 and mark the new fd in fdtable. If the fd *
! * is already >= 10, it is not moved. If it is invalid, -1 is returned. */
/**/
int
movefd(int fd)
{
! if(fd != -1 && fd < 10) {
#ifdef F_DUPFD
! int fe = fcntl(fd, F_DUPFD, 10);
#else
! int fe = movefd(dup(fd));
#endif
! close(fd);
! fdtable[fd] = 0;
! fd = fe;
! }
! if(fd != -1)
! fdtable[fd] = 1;
! return fd;
}
! /* Move fd x to y. If x == -1, fd y is closed. */
/**/
void
redup(int x, int y)
{
! if(x < 0) {
! close(y);
! fdtable[y] = 0;
! } else if (x != y) {
dup2(x, y);
+ fdtable[y] = fdtable[x];
close(x);
fdtable[x] = 0;
}
}
! /* Close the given fd, and clear it from fdtable. */
/**/
int
zclose(int fd)
{
! if (fd >= 0)
fdtable[fd] = 0;
! return close(fd);
}
/* Get a file name relative to $TMPPREFIX which *
-----BEGIN PGP SIGNATURE-----
Version: 2.6.2
iQCVAwUBMZKI43D/+HJTpU/hAQFHNAP9HawGu40RFKgRwTyD9t7Dyuarkm+QrEVQ
H/VPOjZXWTPfKyFtwUTL5eu6iHDtbzYKNxJKSMCQg26krIQW0SSwpQoDiL++wFOG
/yfDaYX0FhxJjH2k/JEL9DoT8B2SDLlZ0Ac2s7iOhKuF9bxrEe4Q99FUE65M+0Yo
MrpE1Ltg5WA=
=PYae
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~1996-05-10 10:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1996-05-08 23:50 fdtable bug Zefram
1996-05-10 9:38 ` Zefram
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
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).