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