Re: CVS commit: src/sys/net

[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?



On Wed, 5 Apr 2017, Ryota Ozaki wrote:

> Module Name:	src
> Committed By:	ozaki-r
> Date:		Wed Apr  5 03:47:51 UTC 2017
>
> Modified Files:
> 	src/sys/net: if.c if.h if_ethersubr.c link_proto.c
>
> Log Message:
> Make sure to hold if_ioctl_lock when calling ifp->if_ioctl
>
> Unfortunately callers of ifp->if_ioctl (if_addr_init, if_flags_set
> and if_mcast_op) may or may not hold if_ioctl_lock, so we have to
> hold the lock only if it's not held.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.389 -r1.390 src/sys/net/if.c
> cvs rdiff -u -r1.236 -r1.237 src/sys/net/if.h
> cvs rdiff -u -r1.240 -r1.241 src/sys/net/if_ethersubr.c
> cvs rdiff -u -r1.34 -r1.35 src/sys/net/link_proto.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>
> !DSPAM:58e49ead273461966420671!

+------------------+--------------------------+----------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:          |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+------------------+--------------------------+----------------------------+