Avoid double rt_replace_ifa on rtrequest1(RTM_ADD)
Some callers of rtrequest1(RTM_ADD) adjust rt_ifa of an rtentry created by rtrequest1 that may change rt_ifa (in ifa_rtrequest) with another ifa that is different from requested one. It's wasteful and even worse introduces a race condition. rtrequest1 should just use a passed ifa as is if a caller hopes so.
This commit is contained in:
ozaki-r
parent
954d82d49e
commit
973496ef18
10
sys/net/if.c
10
sys/net/if.c
@ -1,4 +1,4 @@
|
||||
/* $NetBSD: if.c,v 1.439 2018/10/30 05:29:21 ozaki-r Exp $ */
|
||||
/* $NetBSD: if.c,v 1.440 2018/10/30 05:54:42 ozaki-r Exp $ */
|
||||
|
||||
/*-
|
||||
* Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
|
||||
@ -90,7 +90,7 @@
|
||||
*/
|
||||
|
||||
#include <sys/cdefs.h>
|
||||
__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.439 2018/10/30 05:29:21 ozaki-r Exp $");
|
||||
__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.440 2018/10/30 05:54:42 ozaki-r Exp $");
|
||||
|
||||
#if defined(_KERNEL_OPT)
|
||||
#include "opt_inet.h"
|
||||
@ -2183,7 +2183,8 @@ link_rtrequest(int cmd, struct rtentry *rt, const struct rt_addrinfo *info)
|
||||
struct psref psref;
|
||||
|
||||
if (cmd != RTM_ADD || (ifa = rt->rt_ifa) == NULL ||
|
||||
(ifp = ifa->ifa_ifp) == NULL || (dst = rt_getkey(rt)) == NULL)
|
||||
(ifp = ifa->ifa_ifp) == NULL || (dst = rt_getkey(rt)) == NULL ||
|
||||
ISSET(info->rti_flags, RTF_DONTCHANGEIFA))
|
||||
return;
|
||||
if ((ifa = ifaof_ifpforaddr_psref(dst, ifp, &psref)) != NULL) {
|
||||
rt_replace_ifa(rt, ifa);
|
||||
@ -2437,6 +2438,9 @@ p2p_rtrequest(int req, struct rtentry *rt,
|
||||
|
||||
rt->rt_ifp = lo0ifp;
|
||||
|
||||
if (ISSET(info->rti_flags, RTF_DONTCHANGEIFA))
|
||||
break;
|
||||
|
||||
IFADDR_READER_FOREACH(ifa, ifp) {
|
||||
if (equal(rt_getkey(rt), ifa->ifa_addr))
|
||||
break;
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
/* $NetBSD: route.c,v 1.214 2018/10/30 05:30:31 ozaki-r Exp $ */
|
||||
/* $NetBSD: route.c,v 1.215 2018/10/30 05:54:42 ozaki-r Exp $ */
|
||||
|
||||
/*-
|
||||
* Copyright (c) 1998, 2008 The NetBSD Foundation, Inc.
|
||||
@ -97,7 +97,7 @@
|
||||
#endif
|
||||
|
||||
#include <sys/cdefs.h>
|
||||
__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.214 2018/10/30 05:30:31 ozaki-r Exp $");
|
||||
__KERNEL_RCSID(0, "$NetBSD: route.c,v 1.215 2018/10/30 05:54:42 ozaki-r Exp $");
|
||||
|
||||
#include <sys/param.h>
|
||||
#ifdef RTFLUSH_DEBUG
|
||||
@ -1242,7 +1242,7 @@ rtrequest1(int req, struct rt_addrinfo *info, struct rtentry **ret_nrt)
|
||||
if (rt == NULL)
|
||||
senderr(ENOBUFS);
|
||||
memset(rt, 0, sizeof(*rt));
|
||||
rt->rt_flags = RTF_UP | flags;
|
||||
rt->rt_flags = RTF_UP | (flags & ~RTF_DONTCHANGEIFA);
|
||||
LIST_INIT(&rt->rt_timer);
|
||||
|
||||
RT_DPRINTF("rt->_rt_key = %p\n", (void *)rt->_rt_key);
|
||||
@ -1605,7 +1605,7 @@ rtinit(struct ifaddr *ifa, int cmd, int flags)
|
||||
}
|
||||
memset(&info, 0, sizeof(info));
|
||||
info.rti_ifa = ifa;
|
||||
info.rti_flags = flags | ifa->ifa_flags;
|
||||
info.rti_flags = flags | ifa->ifa_flags | RTF_DONTCHANGEIFA;
|
||||
info.rti_info[RTAX_DST] = dst;
|
||||
info.rti_info[RTAX_GATEWAY] = ifa->ifa_addr;
|
||||
|
||||
@ -1636,40 +1636,7 @@ rtinit(struct ifaddr *ifa, int cmd, int flags)
|
||||
rt_unref(rt);
|
||||
break;
|
||||
case RTM_ADD:
|
||||
/*
|
||||
* XXX it looks just reverting rt_ifa replaced by ifa_rtrequest
|
||||
* called via rtrequest1. Can we just prevent the replacement
|
||||
* somehow and remove the following code? And also doesn't
|
||||
* calling ifa_rtrequest(RTM_ADD) replace rt_ifa again?
|
||||
*/
|
||||
if (rt->rt_ifa != ifa) {
|
||||
printf("rtinit: wrong ifa (%p) was (%p)\n", ifa,
|
||||
rt->rt_ifa);
|
||||
#ifdef NET_MPSAFE
|
||||
KASSERT(!cpu_softintr_p());
|
||||
|
||||
error = rt_update_prepare(rt);
|
||||
if (error == 0) {
|
||||
#endif
|
||||
if (rt->rt_ifa->ifa_rtrequest != NULL) {
|
||||
rt->rt_ifa->ifa_rtrequest(RTM_DELETE,
|
||||
rt, &info);
|
||||
}
|
||||
rt_replace_ifa(rt, ifa);
|
||||
rt->rt_ifp = ifa->ifa_ifp;
|
||||
if (ifa->ifa_rtrequest != NULL)
|
||||
ifa->ifa_rtrequest(RTM_ADD, rt, &info);
|
||||
#ifdef NET_MPSAFE
|
||||
rt_update_finish(rt);
|
||||
} else {
|
||||
/*
|
||||
* If error != 0, the rtentry is being
|
||||
* destroyed, so doing nothing doesn't
|
||||
* matter.
|
||||
*/
|
||||
}
|
||||
#endif
|
||||
}
|
||||
KASSERT(rt->rt_ifa == ifa);
|
||||
rt_newmsg(cmd, rt);
|
||||
rt_unref(rt);
|
||||
RT_REFCNT_TRACE(rt);
|
||||
@ -1701,17 +1668,16 @@ rt_ifa_addlocal(struct ifaddr *ifa)
|
||||
struct rtentry *nrt;
|
||||
|
||||
memset(&info, 0, sizeof(info));
|
||||
info.rti_flags = RTF_HOST | RTF_LOCAL;
|
||||
info.rti_flags = RTF_HOST | RTF_LOCAL | RTF_DONTCHANGEIFA;
|
||||
info.rti_info[RTAX_DST] = ifa->ifa_addr;
|
||||
info.rti_info[RTAX_GATEWAY] =
|
||||
(const struct sockaddr *)ifa->ifa_ifp->if_sadl;
|
||||
info.rti_ifa = ifa;
|
||||
nrt = NULL;
|
||||
e = rtrequest1(RTM_ADD, &info, &nrt);
|
||||
if (nrt && ifa != nrt->rt_ifa)
|
||||
rt_replace_ifa(nrt, ifa);
|
||||
rt_newaddrmsg(RTM_ADD, ifa, e, nrt);
|
||||
if (nrt != NULL) {
|
||||
KASSERT(nrt->rt_ifa == ifa);
|
||||
#ifdef RT_DEBUG
|
||||
dump_rt(nrt);
|
||||
#endif
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
/* $NetBSD: route.h,v 1.119 2018/04/19 21:20:43 christos Exp $ */
|
||||
/* $NetBSD: route.h,v 1.120 2018/10/30 05:54:42 ozaki-r Exp $ */
|
||||
|
||||
/*
|
||||
* Copyright (c) 1980, 1986, 1993
|
||||
@ -172,6 +172,11 @@ struct ortentry {
|
||||
#define RTF_LOCAL 0x40000 /* route represents a local address */
|
||||
#define RTF_BROADCAST 0x80000 /* route represents a bcast address */
|
||||
#define RTF_UPDATING 0x100000 /* route is updating */
|
||||
/*
|
||||
* The flag is nevert set to rt_flags. It just tells rtrequest1 to set a passed
|
||||
* ifa to rt_ifa (via rti_ifa) and not replace rt_ifa in ifa_rtrequest.
|
||||
*/
|
||||
#define RTF_DONTCHANGEIFA 0x200000 /* suppress rt_ifa replacement */
|
||||
|
||||
/*
|
||||
* 0x400 is exposed to userland just for backward compatibility. For that
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
/* $NetBSD: if_arp.c,v 1.275 2018/05/11 13:56:43 maxv Exp $ */
|
||||
/* $NetBSD: if_arp.c,v 1.276 2018/10/30 05:54:41 ozaki-r Exp $ */
|
||||
|
||||
/*
|
||||
* Copyright (c) 1998, 2000, 2008 The NetBSD Foundation, Inc.
|
||||
@ -68,7 +68,7 @@
|
||||
*/
|
||||
|
||||
#include <sys/cdefs.h>
|
||||
__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.275 2018/05/11 13:56:43 maxv Exp $");
|
||||
__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.276 2018/10/30 05:54:41 ozaki-r Exp $");
|
||||
|
||||
#ifdef _KERNEL_OPT
|
||||
#include "opt_ddb.h"
|
||||
@ -605,6 +605,11 @@ arp_rtrequest(int req, struct rtentry *rt, const struct rt_addrinfo *info)
|
||||
rt->rt_rmx.rmx_mtu = 0;
|
||||
}
|
||||
rt->rt_flags |= RTF_LOCAL;
|
||||
|
||||
if (ISSET(info->rti_flags, RTF_DONTCHANGEIFA)) {
|
||||
pserialize_read_exit(s);
|
||||
goto out;
|
||||
}
|
||||
/*
|
||||
* make sure to set rt->rt_ifa to the interface
|
||||
* address we are using, otherwise we will have trouble
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
/* $NetBSD: nd6.c,v 1.250 2018/09/03 16:29:36 riastradh Exp $ */
|
||||
/* $NetBSD: nd6.c,v 1.251 2018/10/30 05:54:41 ozaki-r Exp $ */
|
||||
/* $KAME: nd6.c,v 1.279 2002/06/08 11:16:51 itojun Exp $ */
|
||||
|
||||
/*
|
||||
@ -31,7 +31,7 @@
|
||||
*/
|
||||
|
||||
#include <sys/cdefs.h>
|
||||
__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.250 2018/09/03 16:29:36 riastradh Exp $");
|
||||
__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.251 2018/10/30 05:54:41 ozaki-r Exp $");
|
||||
|
||||
#ifdef _KERNEL_OPT
|
||||
#include "opt_net_mpsafe.h"
|
||||
@ -1566,7 +1566,8 @@ nd6_rtrequest(int req, struct rtentry *rt, const struct rt_addrinfo *info)
|
||||
* that the rt_ifa points to the address instead
|
||||
* of the loopback address.
|
||||
*/
|
||||
if (ifa != rt->rt_ifa)
|
||||
if (!ISSET(info->rti_flags, RTF_DONTCHANGEIFA)
|
||||
&& ifa != rt->rt_ifa)
|
||||
rt_replace_ifa(rt, ifa);
|
||||
}
|
||||
} else if (rt->rt_flags & RTF_ANNOUNCE) {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user