Subject: F_SETOWN/TIOCGPGRP/FIOSETOWN handling
To: None <tech-kern@netbsd.org>
From: Jaromir Dolecek <jdolecek@netbsd.org>
List: tech-kern
Date: 09/13/2003 19:59:04
While looking at kern/22042 I realized SETOWN handling is
a bit suboptimal.
* any F_SETOWN/FIOSETOWN is converted to TIOCSPGRP request and
'owner' is set to process group of the process; sockets are
handled specially and they are the only type of object which can
have single process as 'owner'
* the handling of TIOCSPGRP and SIGIO delivery is done separately
for each type of device or object supporting it; each place used
it's own variant of
if (pgid < 0)
gsignal(-pgid, SIGIO);
else if (p = pfind(pgid))
psignal(p, SIGIO);
* FIOGETOWN ioctl doesn't seem to actually work - it doesn't
copy the data back to userland if I read the code correctly
I'd like to consolidate this a bit, and came up with these
changes:
* FIOSETOWN ioctl is now passed down to file object, just like
TIOCSPGRP; the translation in generic code is removed
* there are new fsetown(), fgetown(), fownsignal() which
handle the ioctls, setting of object's pgid and signal
delivery - callers pass pointer to place where the
process/group ID is stored, and don't interpret the value
in any way
* special handling of sockets in generic code is removed,
the ioctls go down to soo_ioctl() and are handled there
This makes the handling totally opaque for leaf objects,
making the code easier to read and less error prone.
Here is part of patch for sys/kern, sys/sys/ and sys/net/.
Opinions?
Jaromir
Index: kern/kern_descrip.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_descrip.c,v
retrieving revision 1.112
diff -u -p -r1.112 kern_descrip.c
--- kern/kern_descrip.c 2003/09/13 08:32:13 1.112
+++ kern/kern_descrip.c 2003/09/13 17:52:19
@@ -352,31 +352,11 @@ sys_fcntl(struct lwp *l, void *v, regist
break;
case F_GETOWN:
- if (fp->f_type == DTYPE_SOCKET) {
- *retval = ((struct socket *)fp->f_data)->so_pgid;
- goto out;
- }
- error = (*fp->f_ops->fo_ioctl)(fp, TIOCGPGRP, &tmp, p);
- *retval = -tmp;
+ error = (*fp->f_ops->fo_ioctl)(fp, FIOGETOWN, retval, p);
break;
case F_SETOWN:
- if (fp->f_type == DTYPE_SOCKET) {
- ((struct socket *)fp->f_data)->so_pgid =
- (long)SCARG(uap, arg);
- goto out;
- }
- if ((long)SCARG(uap, arg) <= 0) {
- tmp = (-(long)SCARG(uap, arg));
- } else {
- struct proc *p1 = pfind((long)SCARG(uap, arg));
- if (p1 == 0) {
- error = ESRCH;
- goto out;
- }
- tmp = (long)p1->p_pgrp->pg_id;
- }
- error = (*fp->f_ops->fo_ioctl)(fp, TIOCSPGRP, &tmp, p);
+ error = (*fp->f_ops->fo_ioctl)(fp, FIOSETOWN, retval, p);
break;
case F_SETLKW:
@@ -1550,3 +1530,51 @@ restart:
return (0);
}
#undef CHECK_UPTO
+
+int
+fsetown(struct proc *p, pid_t *pgid, int cmd, const void *data)
+{
+ int id = *(int *)data;
+ int error;
+
+ if (cmd == TIOCSPGRP) {
+ if (id < 0)
+ return (EINVAL);
+ id = -id;
+ }
+
+ if (id > 0 && !pfind(id))
+ return (ESRCH);
+ else if (id < 0 && (error = pgid_in_session(p, -id)))
+ return (error);
+
+ *pgid = id;
+ return (0);
+}
+
+int
+fgetown(struct proc *p, pid_t *pgid, int cmd, void *data)
+{
+ if (cmd == TIOCGPGRP)
+ *(int *)data = -*pgid;
+ else
+ *(int *)data = *pgid;
+ return (0);
+}
+
+void
+fownsignal(pid_t pgid, int code, int band, void *fdescdata)
+{
+ struct proc *p1;
+ ksiginfo_t ksi;
+
+ memset(&ksi, 0, sizeof(ksi));
+ ksi.ksi_signo = SIGIO;
+ ksi.ksi_code = code;
+ ksi.ksi_band = band;
+
+ if (pgid > 0 && (p1 = pfind(pgid)))
+ kpsignal(p1, &ksi, fdescdata);
+ else if (pgid < 0)
+ kgsignal(-pgid, &ksi, fdescdata);
+}
Index: kern/subr_log.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_log.c,v
retrieving revision 1.31
diff -u -p -r1.31 subr_log.c
--- kern/subr_log.c 2003/09/07 09:31:47 1.31
+++ kern/subr_log.c 2003/09/13 17:52:20
@@ -59,7 +59,7 @@ __KERNEL_RCSID(0, "$NetBSD: subr_log.c,v
struct logsoftc {
int sc_state; /* see above for possibilities */
struct selinfo sc_selp; /* process waiting on select call */
- int sc_pgid; /* process/group for async I/O */
+ pid_t sc_pgid; /* process/group for async I/O */
} logsoftc;
int log_open; /* also used in log() */
@@ -277,18 +277,11 @@ logkqfilter(dev_t dev, struct knote *kn)
void
logwakeup()
{
- struct proc *p;
-
if (!log_open)
return;
selnotify(&logsoftc.sc_selp, 0);
- if (logsoftc.sc_state & LOG_ASYNC) {
- if (logsoftc.sc_pgid < 0)
- gsignal(-logsoftc.sc_pgid, SIGIO);
- else if (logsoftc.sc_pgid > 0 &&
- (p = pfind(logsoftc.sc_pgid)) != NULL)
- psignal(p, SIGIO);
- }
+ if (logsoftc.sc_state & LOG_ASYNC)
+ fownsignal(logsoftc.sc_pgid, 0, 0, NULL);
if (logsoftc.sc_state & LOG_RDWAIT) {
wakeup((caddr_t)msgbufp);
logsoftc.sc_state &= ~LOG_RDWAIT;
@@ -306,8 +299,6 @@ logioctl(dev, com, data, flag, p)
{
long l;
int s;
- pid_t pgid;
- int error;
switch (com) {
@@ -332,18 +323,12 @@ logioctl(dev, com, data, flag, p)
break;
case TIOCSPGRP:
- pgid = *(int *)data;
- if (pgid != 0) {
- error = pgid_in_session(p, pgid);
- if (error)
- return error;
- }
- logsoftc.sc_pgid = -pgid;
- break;
+ case FIOSETOWN:
+ return fsetown(p, &logsoftc.sc_pgid, com, data);
case TIOCGPGRP:
- *(int *)data = -logsoftc.sc_pgid;
- break;
+ case FIOGETOWN:
+ return fgetown(p, &logsoftc.sc_pgid, com, data);
default:
return (EPASSTHROUGH);
Index: kern/sys_generic.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_generic.c,v
retrieving revision 1.77
diff -u -p -r1.77 sys_generic.c
--- kern/sys_generic.c 2003/08/07 16:31:54 1.77
+++ kern/sys_generic.c 2003/09/13 17:52:20
@@ -609,38 +609,6 @@ sys_ioctl(struct lwp *l, void *v, regist
error = (*fp->f_ops->fo_ioctl)(fp, FIOASYNC, (caddr_t)&tmp, p);
break;
- case FIOSETOWN:
- tmp = *(int *)data;
- if (fp->f_type == DTYPE_SOCKET) {
- ((struct socket *)fp->f_data)->so_pgid = tmp;
- error = 0;
- break;
- }
- if (tmp <= 0) {
- tmp = -tmp;
- } else {
- struct proc *p1 = pfind(tmp);
- if (p1 == 0) {
- error = ESRCH;
- break;
- }
- tmp = p1->p_pgrp->pg_id;
- }
- error = (*fp->f_ops->fo_ioctl)
- (fp, TIOCSPGRP, (caddr_t)&tmp, p);
- break;
-
- case FIOGETOWN:
- if (fp->f_type == DTYPE_SOCKET) {
- error = 0;
- *(int *)data = ((struct socket *)fp->f_data)->so_pgid;
- break;
- }
- error = (*fp->f_ops->fo_ioctl)(fp, TIOCGPGRP, data, p);
- if (error == 0)
- *(int *)data = -*(int *)data;
- break;
-
default:
error = (*fp->f_ops->fo_ioctl)(fp, com, data, p);
/*
Index: kern/sys_pipe.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_pipe.c,v
retrieving revision 1.41
diff -u -p -r1.41 sys_pipe.c
--- kern/sys_pipe.c 2003/08/11 10:24:41 1.41
+++ kern/sys_pipe.c 2003/09/13 17:52:22
@@ -399,21 +399,9 @@ static void
pipeselwakeup(selp, sigp)
struct pipe *selp, *sigp;
{
- struct proc *p;
- pid_t pid;
-
selnotify(&selp->pipe_sel, 0);
- if (sigp == NULL || (sigp->pipe_state & PIPE_ASYNC) == 0)
- return;
-
- pid = sigp->pipe_pgid;
- if (pid == 0)
- return;
-
- if (pid > 0)
- gsignal(pid, SIGIO);
- else if ((p = pfind(-pid)) != NULL)
- psignal(p, SIGIO);
+ if (sigp != NULL && (sigp->pipe_state & PIPE_ASYNC))
+ fownsignal(sigp->pipe_pgid, 0, 0, selp);
}
/* ARGSUSED */
@@ -1092,8 +1080,6 @@ pipe_ioctl(fp, cmd, data, p)
struct proc *p;
{
struct pipe *pipe = (struct pipe *)fp->f_data;
- pid_t pgid;
- int error;
switch (cmd) {
@@ -1122,18 +1108,12 @@ pipe_ioctl(fp, cmd, data, p)
return (0);
case TIOCSPGRP:
- pgid = *(int *)data;
- if (pgid != 0) {
- error = pgid_in_session(p, pgid);
- if (error)
- return error;
- }
- pipe->pipe_pgid = pgid;
- return (0);
+ case FIOSETOWN:
+ return fsetown(p, &pipe->pipe_pgid, cmd, data);
case TIOCGPGRP:
- *(int *)data = pipe->pipe_pgid;
- return (0);
+ case FIOGETOWN:
+ return fgetown(p, &pipe->pipe_pgid, cmd, data);
}
return (EPASSTHROUGH);
Index: kern/sys_socket.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_socket.c,v
retrieving revision 1.38
diff -u -p -r1.38 sys_socket.c
--- kern/sys_socket.c 2003/08/07 16:31:55 1.38
+++ kern/sys_socket.c 2003/09/13 17:52:22
@@ -117,12 +117,14 @@ soo_ioctl(fp, cmd, data, p)
return (0);
case SIOCSPGRP:
- so->so_pgid = *(int *)data;
- return (0);
+ case FIOSETOWN:
+ case TIOCSPGRP:
+ return fsetown(p, &so->so_pgid, cmd, data);
case SIOCGPGRP:
- *(int *)data = so->so_pgid;
- return (0);
+ case FIOGETOWN:
+ case TIOCGPGRP:
+ return fgetown(p, &so->so_pgid, cmd, data);
case SIOCATMARK:
*(int *)data = (so->so_state&SS_RCVATMARK) != 0;
Index: kern/tty.c
===================================================================
RCS file: /cvsroot/src/sys/kern/tty.c,v
retrieving revision 1.156
diff -u -p -r1.156 tty.c
--- kern/tty.c 2003/08/11 10:49:06 1.156
+++ kern/tty.c 2003/09/13 17:52:25
@@ -782,6 +782,7 @@ ttioctl(struct tty *tp, u_long cmd, cadd
case TIOCSETAW:
#ifdef notdef
case TIOCSPGRP:
+ case FIOSETOWN:
#endif
case TIOCSTAT:
case TIOCSTI:
@@ -893,6 +894,11 @@ ttioctl(struct tty *tp, u_long cmd, cadd
case TIOCGWINSZ: /* get window size */
*(struct winsize *)data = tp->t_winsize;
break;
+ case FIOGETOWN:
+ if (!isctty(p, tp))
+ return (ENOTTY);
+ *(int *)data = tp->t_pgrp ? -tp->t_pgrp->pg_id : 0;
+ break;
case TIOCGPGRP: /* get pgrp of tty */
if (!isctty(p, tp))
return (ENOTTY);
@@ -1087,6 +1093,29 @@ ttioctl(struct tty *tp, u_long cmd, cadd
p->p_session->s_ttyp = tp;
p->p_flag |= P_CONTROLT;
break;
+ case FIOSETOWN: { /* set pgrp of tty */
+ pid_t pgid = *(int *)data;
+ struct pgrp *pgrp;
+
+ if (!isctty(p, tp))
+ return (ENOTTY);
+
+ if (pgid < 0)
+ pgrp = pgfind(-pgid);
+ else {
+ struct proc *p1 = pfind(pgid);
+ if (!p1)
+ return (ESRCH);
+ pgrp = p1->p_pgrp;
+ }
+
+ if (pgrp == NULL)
+ return (EINVAL);
+ else if (pgrp->pg_session != p->p_session)
+ return (EPERM);
+ tp->t_pgrp = pgrp;
+ break;
+ }
case TIOCSPGRP: { /* set pgrp of tty */
struct pgrp *pgrp = pgfind(*(int *)data);
Index: kern/uipc_socket2.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.55
diff -u -p -r1.55 uipc_socket2.c
--- kern/uipc_socket2.c 2003/09/06 22:03:10 1.55
+++ kern/uipc_socket2.c 2003/09/13 17:52:26
@@ -303,8 +303,6 @@ sb_lock(struct sockbuf *sb)
void
sowakeup(struct socket *so, struct sockbuf *sb, int code)
{
- struct proc *p;
-
selnotify(&sb->sb_sel, 0);
sb->sb_flags &= ~SB_SEL;
if (sb->sb_flags & SB_WAIT) {
@@ -312,25 +310,19 @@ sowakeup(struct socket *so, struct sockb
wakeup((caddr_t)&sb->sb_cc);
}
if (sb->sb_flags & SB_ASYNC) {
- ksiginfo_t ksi;
- memset(&ksi, 0, sizeof(ksi));
- ksi.ksi_signo = SIGIO;
- ksi.ksi_code = code;
+ int band;
if (code == POLL_IN) {
if (so->so_oobmark || (so->so_state & SS_RCVATMARK))
- ksi.ksi_band = (POLLPRI | POLLRDBAND);
+ band = (POLLPRI | POLLRDBAND);
else
- ksi.ksi_band = (POLLIN | POLLRDNORM);
+ band = (POLLIN | POLLRDNORM);
} else {
if (so->so_oobmark)
- ksi.ksi_band = (POLLPRI | POLLWRBAND);
+ band = (POLLPRI | POLLWRBAND);
else
- ksi.ksi_band = (POLLOUT | POLLWRNORM);
+ band = (POLLOUT | POLLWRNORM);
}
- if (so->so_pgid < 0)
- kgsignal(-so->so_pgid, &ksi, so);
- else if (so->so_pgid > 0 && (p = pfind(so->so_pgid)) != 0)
- kpsignal(p, &ksi, so);
+ fownsignal(so->so_pgid, code, band, so);
}
if (sb->sb_flags & SB_UPCALL)
(*so->so_upcall)(so, so->so_upcallarg, M_DONTWAIT);
Index: sys/file.h
===================================================================
RCS file: /cvsroot/src/sys/sys/file.h,v
retrieving revision 1.46
diff -u -p -r1.46 file.h
--- sys/file.h 2003/08/07 16:34:03 1.46
+++ sys/file.h 2003/09/13 17:52:26
@@ -162,6 +162,10 @@ int dofilewritev(struct proc *, int, str
void finit(void);
+int fsetown(struct proc *, pid_t *, int, const void *);
+int fgetown(struct proc *, pid_t *, int, void *);
+void fownsignal(pid_t, int, int, void *);
+
#endif /* _KERNEL */
#endif /* _SYS_FILE_H_ */
Index: net/bpf.c
===================================================================
RCS file: /cvsroot/src/sys/net/bpf.c,v
retrieving revision 1.84
diff -u -p -r1.84 bpf.c
--- net/bpf.c 2003/08/13 19:44:12 1.84
+++ net/bpf.c 2003/09/13 17:52:31
@@ -518,15 +518,9 @@ static __inline void
bpf_wakeup(d)
struct bpf_d *d;
{
- struct proc *p;
-
wakeup((caddr_t)d);
- if (d->bd_async) {
- if (d->bd_pgid > 0)
- gsignal (d->bd_pgid, SIGIO);
- else if (d->bd_pgid && (p = pfind (-d->bd_pgid)) != NULL)
- psignal (p, SIGIO);
- }
+ if (d->bd_async)
+ fownsignal(d->bd_pgid, 0, 0, NULL);
selnotify(&d->bd_sel, 0);
/* XXX */
@@ -625,7 +619,6 @@ bpfioctl(dev, cmd, addr, flag, p)
{
struct bpf_d *d = &bpf_dtab[minor(dev)];
int s, error = 0;
- pid_t pgid;
#ifdef BPF_KERN_FILTER
struct bpf_insn **p;
#endif
@@ -863,25 +856,14 @@ bpfioctl(dev, cmd, addr, flag, p)
d->bd_async = *(int *)addr;
break;
- /*
- * N.B. ioctl (FIOSETOWN) and fcntl (F_SETOWN) both end up doing
- * the equivalent of a TIOCSPGRP and hence end up here. *However*
- * TIOCSPGRP's arg is a process group if it's positive and a process
- * id if it's negative. This is exactly the opposite of what the
- * other two functions want!
- * There is code in ioctl and fcntl to make the SETOWN calls do
- * a TIOCSPGRP with the pgid of the process if a pid is given.
- */
case TIOCSPGRP: /* Process or group to send signals to */
- pgid = *(int *)addr;
- if (pgid != 0)
- error = pgid_in_session(p, pgid);
- if (error == 0)
- d->bd_pgid = pgid;
+ case FIOSETOWN:
+ error = fsetown(p, &d->bd_pgid, cmd, addr);
break;
case TIOCGPGRP:
- *(int *)addr = d->bd_pgid;
+ case FIOGETOWN:
+ error = fgetown(p, &d->bd_pgid, cmd, addr);
break;
}
return (error);
Index: net/if_tun.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_tun.c,v
retrieving revision 1.63
diff -u -p -r1.63 if_tun.c
--- net/if_tun.c 2003/06/29 22:31:51 1.63
+++ net/if_tun.c 2003/09/13 17:52:31
@@ -177,7 +177,6 @@ tun_clone_destroy(ifp)
struct ifnet *ifp;
{
struct tun_softc *tp = (void *)ifp;
- struct proc *p;
simple_lock(&tun_softc_lock);
simple_lock(&tp->tun_lock);
@@ -188,13 +187,10 @@ tun_clone_destroy(ifp)
if (tp->tun_flags & TUN_RWAIT) {
tp->tun_flags &= ~TUN_RWAIT;
wakeup((caddr_t)tp);
- }
- if (tp->tun_flags & TUN_ASYNC && tp->tun_pgrp) {
- if (tp->tun_pgrp > 0)
- gsignal(tp->tun_pgrp, SIGIO);
- else if ((p = pfind(-tp->tun_pgrp)) != NULL)
- psignal(p, SIGIO);
}
+ if (tp->tun_flags & TUN_ASYNC && tp->tun_pgrp)
+ fownsignal(tp->tun_pgrp, POLL_HUP, 0, NULL);
+
selwakeup(&tp->tun_rsel);
#if NBPFILTER > 0
@@ -435,7 +431,6 @@ tun_output(ifp, m0, dst, rt)
struct rtentry *rt;
{
struct tun_softc *tp = ifp->if_softc;
- struct proc *p;
#ifdef INET
int s;
int error;
@@ -518,13 +513,10 @@ tun_output(ifp, m0, dst, rt)
if (tp->tun_flags & TUN_RWAIT) {
tp->tun_flags &= ~TUN_RWAIT;
wakeup((caddr_t)tp);
- }
- if (tp->tun_flags & TUN_ASYNC && tp->tun_pgrp) {
- if (tp->tun_pgrp > 0)
- gsignal(tp->tun_pgrp, SIGIO);
- else if ((p = pfind(-tp->tun_pgrp)) != NULL)
- psignal(p, SIGIO);
}
+ if (tp->tun_flags & TUN_ASYNC && tp->tun_pgrp)
+ fownsignal(tp->tun_pgrp, POLL_IN, POLLIN|POLLRDNORM, NULL);
+
selnotify(&tp->tun_rsel, 0);
simple_unlock(&tp->tun_lock);
return (0);
@@ -543,8 +535,7 @@ tunioctl(dev, cmd, data, flag, p)
{
int s;
struct tun_softc *tp;
- pid_t pgid;
- int error;
+ int error=0;
tp = tun_find_unit(dev);
@@ -613,17 +604,13 @@ tunioctl(dev, cmd, data, flag, p)
break;
case TIOCSPGRP:
- pgid = *(int *)data;
- if (pgid != 0) {
- error = pgid_in_session(p, pgid);
- if (error != 0)
- return error;
- }
- tp->tun_pgrp = pgid;
+ case FIOSETOWN:
+ error = fsetown(p, &tp->tun_pgrp, cmd, data);
break;
case TIOCGPGRP:
- *(int *)data = tp->tun_pgrp;
+ case FIOGETOWN:
+ error = fgetown(p, &tp->tun_pgrp, cmd, data);
break;
default:
@@ -631,7 +618,7 @@ tunioctl(dev, cmd, data, flag, p)
return (ENOTTY);
}
simple_unlock(&tp->tun_lock);
- return (0);
+ return (error);
}
/*
Index: net/if_tun.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_tun.h,v
retrieving revision 1.10
diff -u -p -r1.10 if_tun.h
--- net/if_tun.h 2001/10/31 20:08:17 1.10
+++ net/if_tun.h 2003/09/13 17:52:31
@@ -36,7 +36,7 @@ struct tun_softc {
#define TUN_READY (TUN_OPEN | TUN_INITED | TUN_IASET)
- int tun_pgrp; /* the process group - if any */
+ pid_t tun_pgrp; /* the process group - if any */
struct selinfo tun_rsel; /* read select */
struct selinfo tun_wsel; /* write select (not used) */
int tun_unit; /* the tunnel unit number */
--
Jaromir Dolecek <jdolecek@NetBSD.org> http://www.NetBSD.cz/
-=- We should be mindful of the potential goal, but as the tantric -=-
-=- Buddhist masters say, ``You may notice during meditation that you -=-
-=- sometimes levitate or glow. Do not let this distract you.'' -=-