Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/sys/net



> Date: Wed, 5 Apr 2017 15:46:38 +0800 (+08)
> From: Paul Goyette <paul%whooppee.com@localhost>
> 
> @@ -3385,6 +3428,13 @@ if_mcast_op(ifnet_t *ifp, const unsigned
>   {
>          int rc;
>          struct ifreq ifr;
> +       bool need_unlock = false;
> +
> +       /* XXX if_ioctl_lock may or may not be held here */
> +       if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
> +               mutex_enter(ifp->if_ioctl_lock);
> +               need_unlock = true;
> +       }
> 
> It is my understanding that using mutex_owned() to effect locking 
> decisions is forbidden.
> 
> I understand the intent of doing it this way, but perhaps we should 
> revisit the callers and enforce that they always take the lock?

Quite right.  Please revert this change and post a note to tech-net@
about how to resolve the issue properly if you need help, including a
statement of what code paths do or don't hold the ioctl lock and why
it's hard to address them.

From what I recall (haven't looked recently, though) the ifioctl
locking may be broken, and may be better written with localcount(9).


Home | Main Index | Thread Index | Old Index