On Thu, Mar 13, 2025 at 11:07:18PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> Maybe shortly before release it is better to throw a kernel lock
> around calls to rtrequest_delete().  It modifies rtentry fields
> documented with
>  *      X       exclusive net lock, or shared net lock + kernel lock
> 
> I the past we were holding exclusive net lock in all network code.
> As this is no longer the case, more kernel lock may safe us.  TCP
> timers doing path MTU discovery have shared net lock only.  Later
> we can figure out which mutexes and locks are necessary in the
> routing code and can remove kernel lock again.
> 

ok mvs.

> bluhm
> 
> Index: net/route.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
> diff -u -p -r1.443 route.c
> --- net/route.c	6 Mar 2025 23:09:02 -0000	1.443
> +++ net/route.c	13 Mar 2025 20:33:54 -0000
> @@ -808,7 +808,9 @@ rtdeletemsg(struct rtentry *rt, struct i
>  	info.rti_flags = rt->rt_flags;
>  	info.rti_info[RTAX_IFP] = sdltosa(ifp->if_sadl);
>  	info.rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
> +	KERNEL_LOCK();
>  	error = rtrequest_delete(&info, rt->rt_priority, ifp, &rt, tableid);
> +	KERNEL_UNLOCK();
>  	rtm_miss(RTM_DELETE, &info, info.rti_flags, rt->rt_priority,
>  	    rt->rt_ifidx, error, tableid);
>  	if (error == 0)
> @@ -1339,7 +1341,9 @@ rt_ifa_del(struct ifaddr *ifa, int flags
>  		prio = ifp->if_priority + RTP_CONNECTED;
>  
>  	rtable_clearsource(rdomain, ifa->ifa_addr);
> +	KERNEL_LOCK();
>  	error = rtrequest_delete(&info, prio, ifp, &rt, rdomain);
> +	KERNEL_UNLOCK();
>  	if (error == 0) {
>  		rtm_send(rt, RTM_DELETE, 0, rdomain);
>  		if (flags & RTF_LOCAL)
> Index: netinet6/nd6_rtr.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6_rtr.c,v
> diff -u -p -r1.171 nd6_rtr.c
> --- netinet6/nd6_rtr.c	14 Jul 2024 18:53:39 -0000	1.171
> +++ netinet6/nd6_rtr.c	13 Mar 2025 20:20:45 -0000
> @@ -204,8 +204,10 @@ rt6_flush(struct in6_addr *gateway, stru
>  			info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
>  			info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt,
>  			    &sa_mask);
> +			KERNEL_LOCK();
>  			error = rtrequest_delete(&info, RTP_ANY, ifp, NULL,
>  			    ifp->if_rdomain);
> +			KERNEL_UNLOCK();
>  			if (error == 0)
>  				error = EAGAIN;
>  		}
>