@@ -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.


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

Reply via email to