From owner-p4-projects@FreeBSD.ORG Fri Jul 28 16:24:21 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id DE84816A510; Fri, 28 Jul 2006 16:24:20 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id B765A16A50D for ; Fri, 28 Jul 2006 16:24:20 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id F1B2E43D7C for ; Fri, 28 Jul 2006 16:24:15 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id k6SGOFEL052781 for ; Fri, 28 Jul 2006 16:24:15 GMT (envelope-from jhb@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id k6SGOFjp052778 for perforce@freebsd.org; Fri, 28 Jul 2006 16:24:15 GMT (envelope-from jhb@freebsd.org) Date: Fri, 28 Jul 2006 16:24:15 GMT Message-Id: <200607281624.k6SGOFjp052778@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jhb@freebsd.org using -f From: John Baldwin To: Perforce Change Reviews Cc: Subject: PERFORCE change 102658 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Jul 2006 16:24:21 -0000 http://perforce.freebsd.org/chv.cgi?CH=102658 Change 102658 by jhb@jhb_mutex on 2006/07/28 16:24:09 - Explicitly lock Giant to protect all of the fields in the stream structure except st_family (read-only field only set when the stream structure is created). This does mean holding it a lot in getmsg() since it wants to read the previous command from putmsg() and then set a new one when it's done. - Mark svr4_sys_ioctl(), svr4_sys_getmsg(), and svr4_sys_putmsg() MPSAFE. Affected files ... .. //depot/projects/smpng/sys/compat/svr4/svr4_stream.c#40 edit .. //depot/projects/smpng/sys/compat/svr4/svr4_stropts.h#4 edit .. //depot/projects/smpng/sys/compat/svr4/syscalls.master#23 edit Differences ... ==== //depot/projects/smpng/sys/compat/svr4/svr4_stream.c#40 (text+ko) ==== @@ -1041,13 +1041,16 @@ return EINVAL; } + mtx_lock(&Giant); if (st->s_afd == -1) { DPRINTF(("fdinsert: accept fd not found\n")); + mtx_unlock(&Giant); return ENOENT; } if ((error = copyin(dat, &fdi, sizeof(fdi))) != 0) { DPRINTF(("fdinsert: copyin failed %d\n", error)); + mtx_unlock(&Giant); return error; } @@ -1057,16 +1060,19 @@ if ((error = dup2(td, &d2p)) != 0) { DPRINTF(("fdinsert: dup2(%d, %d) failed %d\n", st->s_afd, fdi.fd, error)); + mtx_unlock(&Giant); return error; } if ((error = kern_close(td, st->s_afd)) != 0) { DPRINTF(("fdinsert: close(%d) failed %d\n", st->s_afd, error)); + mtx_unlock(&Giant); return error; } st->s_afd = -1; + mtx_unlock(&Giant); *retval = 0; return 0; @@ -1194,6 +1200,7 @@ oflags = td->td_retval[0]; /* update the flags */ + mtx_lock(&Giant); if (dat != NULL) { int mask; @@ -1212,6 +1219,7 @@ flags = oflags & ~O_ASYNC; st->s_eventmask = 0; } + mtx_unlock(&Giant); /* set the new flags, if changed */ if (flags != oflags) { @@ -1236,7 +1244,7 @@ u_long cmd; caddr_t dat; { - int error; + int error, eventmask; if (dat != NULL) { struct svr4_strm *st = svr4_stream_get(fp); @@ -1245,8 +1253,11 @@ DPRINTF(("i_getsig: bad file descriptor\n")); return EINVAL; } - if ((error = copyout(&st->s_eventmask, dat, - sizeof(st->s_eventmask))) != 0) { + mtx_lock(&Giant); + eventmask = st->s_eventmask; + mtx_unlock(&Giant); + if ((error = copyout(&eventmask, dat, + sizeof(eventmask))) != 0) { DPRINTF(("i_getsig: bad eventmask pointer\n")); return error; } @@ -1569,7 +1580,10 @@ return ENOSYS; } - switch (st->s_cmd = sc.cmd) { + mtx_lock(&Giant); + st->s_cmd = sc.cmd; + mtx_unlock(&Giant); + switch (sc.cmd) { case SVR4_TI_CONNECT_REQUEST: /* connect */ { @@ -1701,6 +1715,7 @@ return ENOSYS; } + mtx_lock(&Giant); switch (st->s_cmd) { case SVR4_TI_CONNECT_REQUEST: DPRINTF(("getmsg: TI_CONNECT_REQUEST\n")); @@ -1726,6 +1741,7 @@ error = kern_getpeername(td, uap->fd, &sa, &sasize); if (error) { + mtx_unlock(&Giant); DPRINTF(("getmsg: getpeername failed %d\n", error)); return error; } @@ -1748,6 +1764,7 @@ break; default: + mtx_unlock(&Giant); free(sa, M_SONAME); return ENOSYS; } @@ -1782,6 +1799,7 @@ error = kern_accept(td, uap->fd, &sa, &sasize, &afp); if (error) { + mtx_unlock(&Giant); DPRINTF(("getmsg: accept failed %d\n", error)); return error; } @@ -1814,6 +1832,7 @@ fdclose(td->td_proc->p_fd, afp, st->s_afd, td); fdrop(afp, td); st->s_afd = -1; + mtx_unlock(&Giant); free(sa, M_SONAME); return ENOSYS; } @@ -1832,8 +1851,10 @@ if (ctl.len > sizeof(sc)) ctl.len = sizeof(sc); - if ((error = copyin(ctl.buf, &sc, ctl.len)) != 0) + if ((error = copyin(ctl.buf, &sc, ctl.len)) != 0) { + mtx_unlock(&Giant); return error; + } switch (st->s_family) { case AF_INET: @@ -1847,6 +1868,7 @@ break; default: + mtx_unlock(&Giant); return ENOSYS; } @@ -1862,6 +1884,7 @@ error = kern_recvit(td, uap->fd, &msg, UIO_SYSSPACE, NULL); if (error) { + mtx_unlock(&Giant); DPRINTF(("getmsg: recvit failed %d\n", error)); return error; } @@ -1880,6 +1903,7 @@ break; default: + mtx_unlock(&Giant); return ENOSYS; } @@ -1908,6 +1932,7 @@ ra.buf = dat.buf; ra.nbyte = dat.maxlen; if ((error = read(td, &ra)) != 0) { + mtx_unlock(&Giant); return error; } dat.len = *retval; @@ -1915,6 +1940,7 @@ st->s_cmd = SVR4_TI_SENDTO_REQUEST; break; } + mtx_unlock(&Giant); DPRINTF(("getmsg: Unknown state %x\n", st->s_cmd)); return EINVAL; } @@ -1945,8 +1971,10 @@ fdrop(afp, td); st->s_afd = -1; } + mtx_unlock(&Giant); return (error); } + mtx_unlock(&Giant); if (afp) fdrop(afp, td); ==== //depot/projects/smpng/sys/compat/svr4/svr4_stropts.h#4 (text+ko) ==== @@ -116,12 +116,16 @@ * Our internal state for the stream * For now we keep almost nothing... In the future we can keep more * streams state. + * + * Locking key: + * r - Read only field only set during creation + * G - Giant */ struct svr4_strm { - int s_family; /* socket family */ - int s_cmd; /* last getmsg reply or putmsg request */ - int s_afd; /* last accepted fd; [for fd_insert] */ - int s_eventmask; /* state info from I_SETSIG et al */ + int s_family; /* (r) socket family */ + int s_cmd; /* (G) last getmsg reply or putmsg request */ + int s_afd; /* (G) last accepted fd; [for fd_insert] */ + int s_eventmask; /* (G) state info from I_SETSIG et al */ }; /* ==== //depot/projects/smpng/sys/compat/svr4/syscalls.master#23 (text+ko) ==== @@ -104,7 +104,7 @@ int a3, int a4, int a5); } 53 AUE_NULL MSTD { int svr4_sys_semsys(int what, int a2, \ int a3, int a4, int a5); } -54 AUE_NULL STD { int svr4_sys_ioctl(int fd, u_long com, \ +54 AUE_NULL MSTD { int svr4_sys_ioctl(int fd, u_long com, \ caddr_t data); } 55 AUE_NULL UNIMPL uadmin 56 AUE_NULL UNIMPL exch @@ -141,10 +141,10 @@ 82 AUE_NULL UNIMPL libattach 83 AUE_NULL UNIMPL libdetach 84 AUE_NULL UNIMPL sysfs -85 AUE_NULL STD { int svr4_sys_getmsg(int fd, \ +85 AUE_NULL MSTD { int svr4_sys_getmsg(int fd, \ struct svr4_strbuf *ctl, \ struct svr4_strbuf *dat, int *flags); } -86 AUE_NULL STD { int svr4_sys_putmsg(int fd, \ +86 AUE_NULL MSTD { int svr4_sys_putmsg(int fd, \ struct svr4_strbuf *ctl, \ struct svr4_strbuf *dat, int flags); } 87 AUE_NULL MSTD { int svr4_sys_poll(struct pollfd *fds, \