Aren
/
NetBSD
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "Make sure to hold if_ioctl_lock when calling ifp->if_ioctl" 

As per pgoyette@ and riastradh@ requests; we shouldn't decide to
hold a lock based on if the lock is held or not.
This commit is contained in:
ozaki-r 2017-04-06 03:54:59 +00:00
parent 35cd97ea2e
commit 31d0775c4c
4 changed files with 14 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
sys/net/if.c
View File

@ -1,4 +1,4 @@
/* $NetBSD: if.c,v 1.390 2017/04/05 03:47:51 ozaki-r Exp $ */ /* $NetBSD: if.c,v 1.391 2017/04/06 03:54:59 ozaki-r Exp $ */
/*- /*-
* Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc. * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@ -90,7 +90,7 @@
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.390 2017/04/05 03:47:51 ozaki-r Exp $"); __KERNEL_RCSID(0, "$NetBSD: if.c,v 1.391 2017/04/06 03:54:59 ozaki-r Exp $");
#if defined(_KERNEL_OPT) #if defined(_KERNEL_OPT)
#include "opt_inet.h" #include "opt_inet.h"
@ -2496,15 +2496,8 @@ if_slowtimo(void *arg)
int int
ifpromisc(struct ifnet *ifp, int pswitch) ifpromisc(struct ifnet *ifp, int pswitch)
{ {
int pcount, ret = 0; int pcount, ret;
short nflags; short nflags;
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;
}
pcount = ifp->if_pcount; pcount = ifp->if_pcount;
if (pswitch) { if (pswitch) {
@ -2514,11 +2507,11 @@ ifpromisc(struct ifnet *ifp, int pswitch)
* consult IFF_PROMISC when it is brought up. * consult IFF_PROMISC when it is brought up.
*/ */
if (ifp->if_pcount++ != 0) if (ifp->if_pcount++ != 0)
goto out; return 0;
nflags = ifp->if_flags | IFF_PROMISC; nflags = ifp->if_flags | IFF_PROMISC;
} else { } else {
if (--ifp->if_pcount > 0) if (--ifp->if_pcount > 0)
goto out; return 0;
nflags = ifp->if_flags & ~IFF_PROMISC; nflags = ifp->if_flags & ~IFF_PROMISC;
} }
ret = if_flags_set(ifp, nflags); ret = if_flags_set(ifp, nflags);
@ -2526,10 +2519,6 @@ ifpromisc(struct ifnet *ifp, int pswitch)
if (ret != 0) { if (ret != 0) {
ifp->if_pcount = pcount; ifp->if_pcount = pcount;
} }
out:
if (need_unlock)
mutex_exit(ifp->if_ioctl_lock);
return ret; return ret;
} }
@ -2690,8 +2679,6 @@ ifioctl_common(struct ifnet *ifp, u_long cmd, void *data)
struct ifcapreq *ifcr; struct ifcapreq *ifcr;
struct ifdatareq *ifdr; struct ifdatareq *ifdr;
KASSERT(if_ioctl_locked(ifp));
switch (cmd) { switch (cmd) {
case SIOCSIFCAP: case SIOCSIFCAP:
ifcr = data; ifcr = data;
@ -3069,15 +3056,6 @@ out:
return error; return error;
} }
bool
if_ioctl_locked(struct ifnet *ifp)
{
KASSERT(ifp->if_ioctl_lock != NULL);
return mutex_owned(ifp->if_ioctl_lock);
}
/* /*
* Return interface configuration * Return interface configuration
* of system. List may be used * of system. List may be used
@ -3321,23 +3299,14 @@ int
if_addr_init(ifnet_t *ifp, struct ifaddr *ifa, const bool src) if_addr_init(ifnet_t *ifp, struct ifaddr *ifa, const bool src)
{ {
int rc; int rc;
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;
}
if (ifp->if_initaddr != NULL) if (ifp->if_initaddr != NULL)
rc = (*ifp->if_initaddr)(ifp, ifa, src); rc = (*ifp->if_initaddr)(ifp, ifa, src);
else if (src || else if (src ||
/* FIXME: may not hold if_ioctl_lock */
(rc = (*ifp->if_ioctl)(ifp, SIOCSIFDSTADDR, ifa)) == ENOTTY) (rc = (*ifp->if_ioctl)(ifp, SIOCSIFDSTADDR, ifa)) == ENOTTY)
rc = (*ifp->if_ioctl)(ifp, SIOCINITIFADDR, ifa); rc = (*ifp->if_ioctl)(ifp, SIOCINITIFADDR, ifa);
if (need_unlock)
mutex_exit(ifp->if_ioctl_lock);
return rc; return rc;
} }
@ -3378,13 +3347,6 @@ int
if_flags_set(ifnet_t *ifp, const short flags) if_flags_set(ifnet_t *ifp, const short flags)
{ {
int rc; int rc;
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;
}
if (ifp->if_setflags != NULL) if (ifp->if_setflags != NULL)
rc = (*ifp->if_setflags)(ifp, flags); rc = (*ifp->if_setflags)(ifp, flags);
@ -3402,24 +3364,19 @@ if_flags_set(ifnet_t *ifp, const short flags)
* setting/clearing only IFF_PROMISC if the interface * setting/clearing only IFF_PROMISC if the interface
* isn't IFF_UP. Uphold that tradition. * isn't IFF_UP. Uphold that tradition.
*/ */
if (chgdflags == IFF_PROMISC && (ifp->if_flags & IFF_UP) == 0) { if (chgdflags == IFF_PROMISC && (ifp->if_flags & IFF_UP) == 0)
rc = 0; return 0;
goto out;
}
memset(&ifr, 0, sizeof(ifr)); memset(&ifr, 0, sizeof(ifr));
ifr.ifr_flags = flags & ~IFF_CANTCHANGE; ifr.ifr_flags = flags & ~IFF_CANTCHANGE;
/* FIXME: may not hold if_ioctl_lock */
rc = (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, &ifr); rc = (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, &ifr);
if (rc != 0 && cantflags != 0) if (rc != 0 && cantflags != 0)
ifp->if_flags ^= cantflags; ifp->if_flags ^= cantflags;
} }
out:
if (need_unlock)
mutex_exit(ifp->if_ioctl_lock);
return rc; return rc;
} }
@ -3428,13 +3385,6 @@ if_mcast_op(ifnet_t *ifp, const unsigned long cmd, const struct sockaddr *sa)
{ {
int rc; int rc;
struct ifreq ifr; 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;
}
if (ifp->if_mcastop != NULL) if (ifp->if_mcastop != NULL)
rc = (*ifp->if_mcastop)(ifp, cmd, sa); rc = (*ifp->if_mcastop)(ifp, cmd, sa);
@ -3443,9 +3393,6 @@ if_mcast_op(ifnet_t *ifp, const unsigned long cmd, const struct sockaddr *sa)
rc = (*ifp->if_ioctl)(ifp, cmd, &ifr); rc = (*ifp->if_ioctl)(ifp, cmd, &ifr);
} }
if (need_unlock)
mutex_exit(ifp->if_ioctl_lock);
return rc; return rc;
} }

3
sys/net/if.h
View File

@ -1,4 +1,4 @@
/* $NetBSD: if.h,v 1.237 2017/04/05 03:47:51 ozaki-r Exp $ */ /* $NetBSD: if.h,v 1.238 2017/04/06 03:54:59 ozaki-r Exp $ */
/*- /*-
* Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc. * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@ -968,7 +968,6 @@ int if_do_dad(struct ifnet *);
int if_mcast_op(ifnet_t *, const unsigned long, const struct sockaddr *); int if_mcast_op(ifnet_t *, const unsigned long, const struct sockaddr *);
int if_flags_set(struct ifnet *, const short); int if_flags_set(struct ifnet *, const short);
int if_clone_list(int, char *, int *); int if_clone_list(int, char *, int *);
bool if_ioctl_locked(struct ifnet *);
struct ifnet *ifunit(const char *); struct ifnet *ifunit(const char *);
struct ifnet *if_get(const char *, struct psref *); struct ifnet *if_get(const char *, struct psref *);

6
sys/net/if_ethersubr.c
View File

@ -1,4 +1,4 @@
/* $NetBSD: if_ethersubr.c,v 1.241 2017/04/05 03:47:51 ozaki-r Exp $ */ /* $NetBSD: if_ethersubr.c,v 1.242 2017/04/06 03:54:59 ozaki-r Exp $ */
/* /*
* Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project. * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@ -61,7 +61,7 @@
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.241 2017/04/05 03:47:51 ozaki-r Exp $"); __KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.242 2017/04/06 03:54:59 ozaki-r Exp $");
#ifdef _KERNEL_OPT #ifdef _KERNEL_OPT
#include "opt_inet.h" #include "opt_inet.h"
@ -1351,8 +1351,6 @@ ether_ioctl(struct ifnet *ifp, u_long cmd, void *data)
static const uint8_t zero[ETHER_ADDR_LEN]; static const uint8_t zero[ETHER_ADDR_LEN];
int error; int error;
KASSERT(if_ioctl_locked(ifp));
switch (cmd) { switch (cmd) {
case SIOCINITIFADDR: case SIOCINITIFADDR:
{ {

7
sys/net/link_proto.c
View File

@ -1,4 +1,4 @@
/* $NetBSD: link_proto.c,v 1.35 2017/04/05 03:47:51 ozaki-r Exp $ */ /* $NetBSD: link_proto.c,v 1.36 2017/04/06 03:55:00 ozaki-r Exp $ */
/*- /*-
* Copyright (c) 1982, 1986, 1993 * Copyright (c) 1982, 1986, 1993
@ -32,7 +32,7 @@
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: link_proto.c,v 1.35 2017/04/05 03:47:51 ozaki-r Exp $"); __KERNEL_RCSID(0, "$NetBSD: link_proto.c,v 1.36 2017/04/06 03:55:00 ozaki-r Exp $");
#include <sys/param.h> #include <sys/param.h>
#include <sys/socket.h> #include <sys/socket.h>
@ -143,9 +143,6 @@ link_control(struct socket *so, unsigned long cmd, void *data,
const struct sockaddr_dl *asdl, *nsdl; const struct sockaddr_dl *asdl, *nsdl;
struct psref psref; struct psref psref;
if (ifp != NULL)
KASSERT(if_ioctl_locked(ifp));
switch (cmd) { switch (cmd) {
case SIOCALIFADDR: case SIOCALIFADDR:
case SIOCDLIFADDR: case SIOCDLIFADDR: