diff --git a/minix/servers/vfs/device.c b/minix/servers/vfs/device.c index c76951471..7d34c0479 100644 --- a/minix/servers/vfs/device.c +++ b/minix/servers/vfs/device.c @@ -199,6 +199,35 @@ static cp_grant_id_t make_grant(endpoint_t driver_e, endpoint_t user_e, int op, return gid; } +/*===========================================================================* + * cdev_map * + *===========================================================================*/ +dev_t cdev_map(dev_t dev, struct fproc *rfp) +{ +/* Map the given device number to a real device number, remapping /dev/tty to + * the given process's controlling terminal if it has one. Perform a bounds + * check on the resulting device's major number, and return NO_DEV on failure. + * This function is idempotent but not used that way. + */ + devmajor_t major; + + /* First cover one special case: /dev/tty, the magic device that translates + * to the controlling tty. + */ + if ((major = major(dev)) == CTTY_MAJOR) { + /* No controlling terminal? Fail the request. */ + if (rfp->fp_tty == NO_DEV) return NO_DEV; + + /* Substitute the controlling terminal device. */ + dev = rfp->fp_tty; + major = major(dev); + } + + if (major < 0 || major >= NR_DEVICES) return NO_DEV; + + return dev; +} + /*===========================================================================* * cdev_get * *===========================================================================*/ @@ -207,32 +236,21 @@ static struct dmap *cdev_get(dev_t dev, devminor_t *minor_dev) /* Obtain the dmap structure for the given device, if a valid driver exists for * the major device. Perform redirection for CTTY_MAJOR. */ - devmajor_t major_dev; struct dmap *dp; int slot; - /* First cover one special case: /dev/tty, the magic device that translates - * to the controlling tty. - */ - if (major(dev) == CTTY_MAJOR) { - /* No controlling terminal? Fail the request. */ - if (fp->fp_tty == 0) return(NULL); - - /* Substitute the controlling terminal device. */ - dev = fp->fp_tty; - } + /* Remap /dev/tty as needed. Perform a bounds check on the major number. */ + if ((dev = cdev_map(dev, fp)) == NO_DEV) + return(NULL); /* Determine task dmap. */ - major_dev = major(dev); - if (major_dev < 0 || major_dev >= NR_DEVICES) return(NULL); - - dp = &dmap[major_dev]; + dp = &dmap[major(dev)]; /* See if driver is roughly valid. */ if (dp->dmap_driver == NONE) return(NULL); if (isokendpt(dp->dmap_driver, &slot) != OK) { - printf("VFS: cdev_get: old driver for major %x (%d)\n", major_dev, + printf("VFS: cdev_get: old driver for major %x (%d)\n", major(dev), dp->dmap_driver); return(NULL); } @@ -531,21 +549,27 @@ int do_ioctl(void) *===========================================================================*/ int cdev_select(dev_t dev, int ops) { -/* Initiate a select call on a device. Return OK iff the request was sent. */ - devminor_t minor_dev; +/* Initiate a select call on a device. Return OK iff the request was sent. + * This function explicitly bypasses cdev_get() since it must not do CTTY + * mapping, because a) the caller already has done that, b) "fp" may be wrong. + */ + devmajor_t major; message dev_mess; struct dmap *dp; int r; - /* Determine task dmap. */ - if ((dp = cdev_get(dev, &minor_dev)) == NULL) - return(EIO); + /* Determine task dmap, without CTTY mapping. */ + assert(dev != NO_DEV); + major = major(dev); + assert(major >= 0 && major < NR_DEVICES); + assert(major != CTTY_MAJOR); + dp = &dmap[major]; /* Prepare the request message. */ memset(&dev_mess, 0, sizeof(dev_mess)); dev_mess.m_type = CDEV_SELECT; - dev_mess.m_vfs_lchardriver_select.minor = minor_dev; + dev_mess.m_vfs_lchardriver_select.minor = minor(dev); dev_mess.m_vfs_lchardriver_select.ops = ops; /* Send the request to the driver. */ diff --git a/minix/servers/vfs/file.h b/minix/servers/vfs/file.h index efbe80d97..a1a4a51e0 100644 --- a/minix/servers/vfs/file.h +++ b/minix/servers/vfs/file.h @@ -29,6 +29,7 @@ EXTERN struct filp { /* following are for fd-type-specific select() */ int filp_pipe_select_ops; + dev_t filp_char_select_dev; } filp[NR_FILPS]; #define FILP_CLOSED 0 /* filp_mode: associated device closed/gone */ diff --git a/minix/servers/vfs/filedes.c b/minix/servers/vfs/filedes.c index a810feb2f..6adc70386 100644 --- a/minix/servers/vfs/filedes.c +++ b/minix/servers/vfs/filedes.c @@ -119,6 +119,7 @@ int get_fd(struct fproc *rfp, int start, mode_t bits, int *k, struct filp **fpt) f->filp_selectors = 0; f->filp_select_ops = 0; f->filp_pipe_select_ops = 0; + f->filp_char_select_dev = NO_DEV; f->filp_flags = 0; f->filp_select_flags = 0; f->filp_softlock = NULL; diff --git a/minix/servers/vfs/misc.c b/minix/servers/vfs/misc.c index 65617d2a3..65d4b8630 100644 --- a/minix/servers/vfs/misc.c +++ b/minix/servers/vfs/misc.c @@ -668,7 +668,7 @@ static void free_proc(int flags) if (vp->v_sdev != dev) continue; lock_filp(rfilp, VNODE_READ); (void) cdev_close(dev); /* Ignore any errors. */ - + /* FIXME: missing select check */ rfilp->filp_mode = FILP_CLOSED; unlock_filp(rfilp); } diff --git a/minix/servers/vfs/proto.h b/minix/servers/vfs/proto.h index ad0449dc9..051a807b6 100644 --- a/minix/servers/vfs/proto.h +++ b/minix/servers/vfs/proto.h @@ -34,6 +34,7 @@ int cdev_open(dev_t dev, int flags); int cdev_close(dev_t dev); int cdev_io(int op, dev_t dev, endpoint_t proc_e, vir_bytes buf, off_t pos, unsigned long bytes, int flags); +dev_t cdev_map(dev_t dev, struct fproc *rfp); int cdev_select(dev_t dev, int ops); int cdev_cancel(dev_t dev); void cdev_reply(void); diff --git a/minix/servers/vfs/select.c b/minix/servers/vfs/select.c index 66ff4ffad..5bf2a1171 100644 --- a/minix/servers/vfs/select.c +++ b/minix/servers/vfs/select.c @@ -57,9 +57,12 @@ static int is_regular_file(struct filp *f); static int is_pipe(struct filp *f); static int is_char_device(struct filp *f); static void select_lock_filp(struct filp *f, int ops); -static int select_request_file(struct filp *f, int *ops, int block); -static int select_request_char(struct filp *f, int *ops, int block); -static int select_request_pipe(struct filp *f, int *ops, int block); +static int select_request_file(struct filp *f, int *ops, int block, + struct fproc *rfp); +static int select_request_char(struct filp *f, int *ops, int block, + struct fproc *rfp); +static int select_request_pipe(struct filp *f, int *ops, int block, + struct fproc *rfp); static void select_cancel_all(struct selectentry *e); static void select_cancel_filp(struct filp *f); static void select_return(struct selectentry *); @@ -68,7 +71,8 @@ static int tab2ops(int fd, struct selectentry *e); static void wipe_select(struct selectentry *s); static struct fdtype { - int (*select_request)(struct filp *, int *ops, int block); + int (*select_request)(struct filp *, int *ops, int block, + struct fproc *rfp); int (*type_match)(struct filp *f); } fdtypes[] = { { select_request_char, is_char_device }, @@ -237,7 +241,7 @@ int do_select(void) wantops = (f->filp_select_ops |= ops); type = se->type[fd]; select_lock_filp(f, wantops); - r = fdtypes[type].select_request(f, &wantops, se->block); + r = fdtypes[type].select_request(f, &wantops, se->block, fp); unlock_filp(f); if (r != OK && r != SUSPEND) { se->error = r; @@ -355,19 +359,48 @@ static int is_char_device(struct filp *f) /*===========================================================================* * select_request_char * *===========================================================================*/ -static int select_request_char(struct filp *f, int *ops, int block) +static int select_request_char(struct filp *f, int *ops, int block, + struct fproc *rfp) { /* Check readiness status on a character device. Unless suitable results are * available right now, this will only initiate the polling process, causing * result processing to be deferred. This function MUST NOT block its calling * thread. The given filp may or may not be locked. */ - devmajor_t major; + dev_t dev; int r, rops; struct dmap *dp; - major = major(f->filp_vno->v_sdev); - if (major < 0 || major >= NR_DEVICES) return(ENXIO); + /* Start by remapping the device node number to a "real" device number. Those + * two are different only for CTTY_MAJOR aka /dev/tty, but that one single + * exception requires quite some extra effort here: the select code matches + * character driver replies to their requests based on the device number, so + * it needs to be aware that device numbers may be mapped. The idea is to + * perform the mapping once and store the result in the filp object, so that + * at least we don't run into problems when a process loses its controlling + * terminal while doing a select (see also free_proc). It should be noted + * that it is possible that multiple processes share the same /dev/tty filp, + * and they may not all have a controlling terminal. The ctty-less processes + * should never pass the mapping; a more problematic case is checked below. + * + * The cdev_map call also checks the major number for rough validity, so that + * we can use it to index the dmap array safely a bit later. + */ + if ((dev = cdev_map(f->filp_vno->v_sdev, rfp)) == NO_DEV) + return(ENXIO); + + if (f->filp_char_select_dev != NO_DEV && f->filp_char_select_dev != dev) { + /* Currently, this case can occur as follows: a process with a + * controlling terminal opens /dev/tty and forks, the new child starts + * a new session, opens a new controlling terminal, and both parent and + * child call select on the /dev/tty file descriptor. If this case ever + * becomes real, a better solution may be to force-close a filp for + * /dev/tty when a new controlling terminal is opened. + */ + printf("VFS: file pointer has multiple controlling TTYs!\n"); + return(EIO); + } + f->filp_char_select_dev = dev; /* set before possibly suspending */ rops = *ops; @@ -401,12 +434,12 @@ static int select_request_char(struct filp *f, int *ops, int block) if (f->filp_select_flags & FSF_BUSY) return(SUSPEND); - dp = &dmap[major]; + dp = &dmap[major(dev)]; if (dp->dmap_sel_busy) return(SUSPEND); f->filp_select_flags &= ~FSF_UPDATE; - r = cdev_select(f->filp_vno->v_sdev, rops); + r = cdev_select(dev, rops); if (r != OK) return(r); @@ -421,7 +454,7 @@ static int select_request_char(struct filp *f, int *ops, int block) * select_request_file * *===========================================================================*/ static int select_request_file(struct filp *UNUSED(f), int *UNUSED(ops), - int UNUSED(block)) + int UNUSED(block), struct fproc *UNUSED(rfp)) { /* Files are always ready, so output *ops is input *ops */ return(OK); @@ -430,7 +463,8 @@ static int select_request_file(struct filp *UNUSED(f), int *UNUSED(ops), /*===========================================================================* * select_request_pipe * *===========================================================================*/ -static int select_request_pipe(struct filp *f, int *ops, int block) +static int select_request_pipe(struct filp *f, int *ops, int block, + struct fproc *UNUSED(rfp)) { /* Check readiness status on a pipe. The given filp is locked. This function * may block its calling thread if necessary. @@ -612,13 +646,15 @@ static void select_cancel_filp(struct filp *f) /* If this filp is the subject of an ongoing select query to a * character device, mark the query as stale, so that this filp will - * not be checked when the result arrives. + * not be checked when the result arrives. The filp select device may + * still be NO_DEV if do_select fails on the initial fd check. */ - if (is_char_device(f)) { - major = major(f->filp_vno->v_sdev); + if (is_char_device(f) && f->filp_char_select_dev != NO_DEV) { + major = major(f->filp_char_select_dev); if (dmap[major].dmap_sel_busy && dmap[major].dmap_sel_filp == f) dmap[major].dmap_sel_filp = NULL; /* leave _busy set */ + f->filp_char_select_dev = NO_DEV; } } } @@ -746,10 +782,11 @@ void select_unsuspend_by_endpt(endpoint_t proc_e) } for (fd = 0; fd < se->nfds; fd++) { - if ((f = se->filps[fd]) == NULL || f->filp_vno == NULL) + if ((f = se->filps[fd]) == NULL || !is_char_device(f)) continue; - major = major(f->filp_vno->v_sdev); + assert(f->filp_char_select_dev != NO_DEV); + major = major(f->filp_char_select_dev); if (dmap_driver_match(proc_e, major)) { se->filps[fd] = NULL; se->error = EIO; @@ -775,7 +812,6 @@ void select_reply1(endpoint_t driver_e, devminor_t minor, int status) dev_t dev; struct filp *f; struct dmap *dp; - struct vnode *vp; /* Figure out which device is replying */ if ((dp = get_dmap(driver_e)) == NULL) return; @@ -796,15 +832,14 @@ void select_reply1(endpoint_t driver_e, devminor_t minor, int status) */ if ((f = dp->dmap_sel_filp) != NULL) { /* Find vnode and check we got a reply from the device we expected */ - vp = f->filp_vno; - assert(vp != NULL); - assert(S_ISCHR(vp->v_mode)); - if (vp->v_sdev != dev) { + assert(is_char_device(f)); + assert(f->filp_char_select_dev != NO_DEV); + if (f->filp_char_select_dev != dev) { /* This should never happen. The driver may be misbehaving. * For now we assume that the reply we want will arrive later.. */ printf("VFS (%s:%d): expected reply from dev %llx not %llx\n", - __FILE__, __LINE__, vp->v_sdev, dev); + __FILE__, __LINE__, f->filp_char_select_dev, dev); return; } } @@ -872,7 +907,6 @@ void select_reply2(endpoint_t driver_e, devminor_t minor, int status) dev_t dev; struct filp *f; struct dmap *dp; - struct vnode *vp; struct selectentry *se; if (status == 0) { @@ -898,9 +932,9 @@ void select_reply2(endpoint_t driver_e, devminor_t minor, int status) found = FALSE; for (fd = 0; fd < se->nfds; fd++) { if ((f = se->filps[fd]) == NULL) continue; - if ((vp = f->filp_vno) == NULL) continue; - if (!S_ISCHR(vp->v_mode)) continue; - if (vp->v_sdev != dev) continue; + if (!is_char_device(f)) continue; + assert(f->filp_char_select_dev != NO_DEV); + if (f->filp_char_select_dev != dev) continue; if (status > 0) { /* Operations ready */ /* Clear the replied bits from the request @@ -974,7 +1008,7 @@ static void select_restart_filps(void) assert(is_char_device(f)); wantops = ops = f->filp_select_ops; - r = select_request_char(f, &wantops, se->block); + r = select_request_char(f, &wantops, se->block, se->requestor); if (r != OK && r != SUSPEND) { se->error = r; restart_proc(se); diff --git a/minix/tests/test77.c b/minix/tests/test77.c index fe0afcbc0..e1e6c7290 100644 --- a/minix/tests/test77.c +++ b/minix/tests/test77.c @@ -482,6 +482,157 @@ test77e(void) if (sigaction(SIGUSR1, &usr_oact, NULL) < 0) e(16); } +/* + * Test basic select functionality on /dev/tty. While this test should not be + * part of this test set, we already have all the infrastructure we need here. + */ +static void +test77f(void) +{ + struct sigaction act, oact; + char c, pname[PATH_MAX], tname[PATH_MAX]; + struct timeval tv; + fd_set fd_set; + int fd, maxfd, masterfd, slavefd; + + subtest = 6; + + /* We do not want to get SIGHUP signals in this test. */ + memset(&act, 0, sizeof(act)); + act.sa_handler = SIG_IGN; + if (sigaction(SIGHUP, &act, &oact) < 0) e(1); + + /* Get master and slave device names for a free pseudo terminal. */ + get_names(pname, tname); + + if ((masterfd = open(pname, O_RDWR | O_NOCTTY)) < 0) e(2); + + switch (fork()) { + case 0: + if (setsid() < 0) e(3); + + close(masterfd); + + if ((slavefd = open(tname, O_RDWR)) < 0) e(4); + + if ((fd = open("/dev/tty", O_RDWR)) < 0) e(5); + + make_raw(fd); + + /* Without slave input, /dev/tty is not ready for reading. */ + FD_ZERO(&fd_set); + FD_SET(fd, &fd_set); + tv.tv_sec = 0; + tv.tv_usec = 0; + + if (select(fd + 1, &fd_set, NULL, NULL, &tv) != 0) e(6); + if (FD_ISSET(fd, &fd_set)) e(7); + + FD_SET(fd, &fd_set); + tv.tv_sec = 0; + tv.tv_usec = 10000; + + if (select(fd + 1, &fd_set, NULL, NULL, &tv) != 0) e(8); + if (FD_ISSET(fd, &fd_set)) e(9); + + /* It will be ready for writing, though. */ + FD_SET(fd, &fd_set); + + if (select(fd + 1, NULL, &fd_set, NULL, NULL) != 1) e(10); + if (!FD_ISSET(fd, &fd_set)) e(11); + + /* Test mixing file descriptors to the same terminal. */ + FD_ZERO(&fd_set); + FD_SET(fd, &fd_set); + FD_SET(slavefd, &fd_set); + tv.tv_sec = 0; + tv.tv_usec = 10000; + + maxfd = fd > slavefd ? fd : slavefd; + if (select(maxfd + 1, &fd_set, NULL, NULL, &tv) != 0) e(12); + if (FD_ISSET(fd, &fd_set)) e(13); + if (FD_ISSET(slavefd, &fd_set)) e(14); + + /* The delayed echo on the master must wake up our select. */ + c = 'A'; + if (write(slavefd, &c, sizeof(c)) != sizeof(c)) e(15); + + FD_ZERO(&fd_set); + FD_SET(fd, &fd_set); + + if (select(fd + 1, &fd_set, NULL, NULL, NULL) != 1) e(16); + if (!FD_ISSET(fd, &fd_set)) e(17); + + /* Select must now still flag readiness for reading. */ + tv.tv_sec = 0; + tv.tv_usec = 0; + + if (select(fd + 1, &fd_set, NULL, NULL, &tv) != 1) e(18); + if (!FD_ISSET(fd, &fd_set)) e(19); + + /* That is, until we read the byte. */ + if (read(slavefd, &c, sizeof(c)) != sizeof(c)) e(20); + if (c != 'B') e(21); + + if (select(fd + 1, &fd_set, NULL, NULL, &tv) != 0) e(22); + if (FD_ISSET(fd, &fd_set)) e(23); + + /* Ask the parent to close the master. */ + c = 'C'; + if (write(slavefd, &c, sizeof(c)) != sizeof(c)) e(24); + + FD_SET(fd, &fd_set); + + /* The closure must cause an EOF condition on the slave. */ + if (select(fd + 1, &fd_set, NULL, NULL, NULL) != 1) e(25); + if (!FD_ISSET(fd, &fd_set)) e(26); + + if (select(fd + 1, &fd_set, NULL, NULL, NULL) != 1) e(27); + if (!FD_ISSET(fd, &fd_set)) e(28); + + if (read(slavefd, &c, sizeof(c)) != 0) e(29); + + exit(errct); + case -1: + e(30); + default: + /* Wait for the child to write something to the slave. */ + FD_ZERO(&fd_set); + FD_SET(masterfd, &fd_set); + + if (select(masterfd + 1, &fd_set, NULL, NULL, NULL) != 1) + e(31); + if (!FD_ISSET(masterfd, &fd_set)) e(32); + + if (read(masterfd, &c, sizeof(c)) != sizeof(c)) e(33); + if (c != 'A') e(34); + + /* Write a reply once the child is blocked in its select. */ + tv.tv_sec = 1; + tv.tv_usec = 0; + if (select(masterfd + 1, &fd_set, NULL, NULL, &tv) != 0) + e(35); + + c = 'B'; + if (write(masterfd, &c, sizeof(c)) != sizeof(c)) e(36); + + /* Wait for the child to request closing the master. */ + if (read(masterfd, &c, sizeof(c)) != sizeof(c)) e(37); + if (c != 'C') e(38); + + /* Close the master once the child is blocked in its select. */ + sleep(1); + + close(masterfd); + + break; + } + + if (waitchild() < 0) e(39); + + if (sigaction(SIGHUP, &oact, NULL) < 0) e(28); +} + int main(int argc, char **argv) { @@ -500,6 +651,7 @@ main(int argc, char **argv) if (m & 0x04) test77c(); if (m & 0x08) test77d(); if (m & 0x10) test77e(); + if (m & 0x20) test77f(); } quit();