Date: Thu, 12 Nov 2009 10:14:12 -0500 From: John Baldwin <jhb@freebsd.org> To: Xin LI <delphij@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r199201 - in head: contrib/libpcap sbin/ifconfig share/man/man4 sys/kern sys/net sys/sys Message-ID: <200911121014.12391.jhb@freebsd.org> In-Reply-To: <200911112130.nABLUw9b007768@svn.freebsd.org> References: <200911112130.nABLUw9b007768@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 11 November 2009 4:30:58 pm Xin LI wrote: > Author: delphij > Date: Wed Nov 11 21:30:58 2009 > New Revision: 199201 > URL: http://svn.freebsd.org/changeset/base/199201 > > Log: > Add interface description capability as inspired by OpenBSD. > > MFC after: 3 months > > Modified: head/sys/net/if.c > ============================================================================== > --- head/sys/net/if.c Wed Nov 11 21:18:27 2009 (r199200) > +++ head/sys/net/if.c Wed Nov 11 21:30:58 2009 (r199201) > @@ -2090,6 +2092,45 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, > ifr->ifr_phys = ifp->if_physical; > break; > > + case SIOCSIFDESCR: > + error = priv_check(td, PRIV_NET_SETIFDESCR); > + if (error) > + return (error); > + > + IF_AFDATA_WLOCK(ifp); > + if (ifp->if_description == NULL) { > + ifp->if_description = sbuf_new_auto(); > + if (ifp->if_description == NULL) { > + error = ENOMEM; > + IF_AFDATA_WUNLOCK(ifp); > + break; > + } > + } else > + sbuf_clear(ifp->if_description); > + > + if (sbuf_copyin(ifp->if_description, ifr->ifr_buffer.buffer, > + ifr->ifr_buffer.length) == -1) > + error = EFAULT; > + > + if (error == 0) { > + sbuf_finish(ifp->if_description); > + getmicrotime(&ifp->if_lastchange); > + } > + IF_AFDATA_WUNLOCK(ifp); Since IF_AFDATA isn't a sleepable lock (e.g. an sx lock), it is not safe to do a copyin() while holding this lock. A better approach would probably be something like: struct sbuf *new, *old; case SIOCSIFDESCR: /* priv check */ new = sbuf_new_auto(); if (new == NULL) return (ENOMEM); if (sbuf_copyin(new, ifr->ifr_buffer.buffer, ifr->ifr_buffer.length) == -1) { sbuf_delete(new); return (EFAULT); } IF_AFDATA_WLOCK(ifp); old = ifp->if_description; ifp->if_description = new; getmicrotime(&ifp->if_lastchange); IF_AFDATA_WUNLOCK(ifp); if (old != NULL) sbuf_delete(old); break; -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911121014.12391.jhb>