On Mon, Apr 17, 2023 at 03:16:36AM +0300, Vitaliy Makkoveev wrote:
> Index: sys/dev/usb/if_umb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 if_umb.c
> --- sys/dev/usb/if_umb.c 31 Mar 2023 23:53:49 -0000 1.50
> +++ sys/dev/usb/if_umb.c 17 Apr 2023 00:09:17 -0000
> @@ -1829,6 +1829,7 @@ umb_add_inet_config(struct umb_softc *sc
> default_sin.sin_len = sizeof (default_sin);
>
> memset(&info, 0, sizeof(info));
> + NET_LOCK();
> info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> info.rti_ifa = ifa_ifwithaddr(sintosa(&ifra.ifra_addr),
> ifp->if_rdomain);
> @@ -1836,7 +1837,6 @@ umb_add_inet_config(struct umb_softc *sc
> info.rti_info[RTAX_NETMASK] = sintosa(&default_sin);
> info.rti_info[RTAX_GATEWAY] = sintosa(&ifra.ifra_dstaddr);
>
> - NET_LOCK();
> rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> NET_UNLOCK();
> if (rv) {
> @@ -1910,6 +1910,7 @@ umb_add_inet6_config(struct umb_softc *s
> default_sin6.sin6_len = sizeof (default_sin6);
>
> memset(&info, 0, sizeof(info));
> + NET_LOCK();
> info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
> ifp->if_rdomain);
> @@ -1917,7 +1918,6 @@ umb_add_inet6_config(struct umb_softc *s
> info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
> info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
>
> - NET_LOCK();
> rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> NET_UNLOCK();
> if (rv) {
This umb chunk is OK bluhm@
> Index: sys/net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.688
> diff -u -p -r1.688 if.c
> --- sys/net/if.c 8 Apr 2023 13:49:38 -0000 1.688
> +++ sys/net/if.c 17 Apr 2023 00:09:17 -0000
> @@ -1409,8 +1409,9 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
> struct ifaddr *ifa;
> u_int rdomain;
>
> + NET_ASSERT_LOCKED();
> +
> rdomain = rtable_l2(rtableid);
> - KERNEL_LOCK();
> TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
> if (ifp->if_rdomain != rdomain)
> continue;
> @@ -1420,12 +1421,10 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
> continue;
>
> if (equal(addr, ifa->ifa_addr)) {
> - KERNEL_UNLOCK();
> return (ifa);
> }
> }
> }
> - KERNEL_UNLOCK();
> return (NULL);
> }
ifa_ifwithdstaddr() is iterating over the same loops. It should
be changed together.
if_unit() is iterating over if_list. There should be a NET_LOCK().
TAILQ_ENTRY(ifnet) if_list; /* [K] all struct ifnets are chained */
TAILQ_ENTRY(ifaddr) ifa_list; /* list of addresses for interface */
Could you put an [N] there?
Can we put an NET_ASSERT_LOCKED_EXCLUSIVE() in ifa_add() and ifa_del()?
> Index: sys/net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.359
> diff -u -p -r1.359 rtsock.c
> --- sys/net/rtsock.c 22 Jan 2023 12:05:44 -0000 1.359
> +++ sys/net/rtsock.c 17 Apr 2023 00:09:17 -0000
> @@ -2374,18 +2374,18 @@ rt_setsource(unsigned int rtableid, stru
> return (EAFNOSUPPORT);
> }
>
> - KERNEL_LOCK();
> + NET_LOCK();
> /*
> * Check if source address is assigned to an interface in the
> * same rdomain
> */
> if ((ifa = ifa_ifwithaddr(src, rtableid)) == NULL) {
> - KERNEL_UNLOCK();
> + NET_UNLOCK();
> return (EINVAL);
> }
>
> error = rtable_setsource(rtableid, src->sa_family, ifa->ifa_addr);
> - KERNEL_UNLOCK();
> + NET_UNLOCK();
>
> return (error);
> }
I would perfer to put the NET_LOCK() into the caller. There are
two more rtable_setsource() in this function which should be net
locked.
