If an RTM_ADD command on a routing socket includes an RTA_IFA sockaddr,
and that sockaddr is an exact match for one of the interfaces in the
relevant routing domain, any RTA_IFP sockaddr in the same message is
ignored.  If there are multiple interfaces with the same IP address,
this can cause packets to be sent out the wrong interface.  Fix this
by skipping the exact-match check on RTA_IFA if an RTA_IFP sockaddr
was sent.

The RTA_IFP sockaddr was also being ignored if there was no interface
with the requested index.  Return ENXIO to userspace instead.

The bug can easily be seen by running the following commands as root
on a machine where vether0 and vether1 do not exist, and on which
the subnet 192.0.2.0/24 (reserved for documentation) is not in use:

    vether1 -ifp vether1 -inet -ifa "$dummy_ip"

On a system with an unpatched kernel, the final command will show
that the route to 192.0.2.6 is via vether0.  If this patch has been
applied, the route to 192.0.2.6 will be (correctly) via vether1.
---
 sys/net/rtsock.c   | 34 ++++++++++++++++++++++++----------
 usr.sbin/arp/arp.c |  4 +---
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git sys/net/rtsock.c sys/net/rtsock.c
index fa84ddc25e5..a156674fa6a 100644
--- sys/net/rtsock.c
+++ sys/net/rtsock.c
@@ -75,6 +75,7 @@
 
 #include <net/if.h>
 #include <net/if_dl.h>
+#include <net/if_types.h>
 #include <net/if_var.h>
 #include <net/route.h>
 
@@ -1228,29 +1229,40 @@ rtm_getifa(struct rt_addrinfo *info, unsigned int rtid)
        NET_ASSERT_LOCKED();
 
        /*
-        * ifp may be specified by sockaddr_dl when protocol address
-        * is ambiguous
+        * ifp may be specified by sockaddr_dl; if so, it must be honored.
         */
        if (info->rti_info[RTAX_IFP] != NULL) {
                struct sockaddr_dl *sdl;
 
                sdl = satosdl(info->rti_info[RTAX_IFP]);
-               ifp = if_get(sdl->sdl_index);
+               if ((ifp = if_get(sdl->sdl_index)) == NULL)
+                       return (ENXIO);
        }
 
 #ifdef IPSEC
        /*
         * If the destination is a PF_KEY address, we'll look
-        * for the existence of a encap interface number or address
-        * in the options list of the gateway. By default, we'll return
-        * enc0.
+        * for the existence of a encap interface number as the output
+        * interface.
         */
        if (info->rti_info[RTAX_DST] &&
-           info->rti_info[RTAX_DST]->sa_family == PF_KEY)
-               info->rti_ifa = enc_getifa(rtid, 0);
+           info->rti_info[RTAX_DST]->sa_family == PF_KEY) {
+               if (ifp != NULL) {
+                       struct enc_softc *sc;
+                       if (ifp->if_type != IFT_ENC) {
+                               if_put(ifp);
+                               return (EINVAL);
+                       }
+                       sc = ifp->if_softc;
+                       info->rti_ifa = &sc->sc_ifa;
+               } else
+                       info->rti_ifa = enc_getifa(rtid, 0);
+       }
 #endif
 
-       if (info->rti_ifa == NULL && info->rti_info[RTAX_IFA] != NULL)
+       /* ifp overrides even an exact match on RTAX_IFA */
+       if (info->rti_ifa == NULL && ifp == NULL &&
+           info->rti_info[RTAX_IFA] != NULL)
                info->rti_ifa = ifa_ifwithaddr(info->rti_info[RTAX_IFA], rtid);
 
        if (info->rti_ifa == NULL) {
@@ -1273,6 +1285,9 @@ rtm_getifa(struct rt_addrinfo *info, unsigned int rtid)
                            sa, sa, rtid);
        }
 
+       KASSERT(info->rti_ifa == NULL || ifp == NULL ||
+           ifp == info->rti_ifa->ifa_ifp);
+
        if_put(ifp);
 
        if (info->rti_ifa == NULL)
@@ -1435,7 +1450,6 @@ rtm_xaddrs(caddr_t cp, caddr_t cplim, struct rt_addrinfo 
*rtinfo)
                        /*
                         * XXX Should be sizeof(struct sockaddr_dl), but
                         * route(8) has a bug and provides less memory.
-                        * arp(8) has another bug and uses sizeof pointer.
                         */
                        size = 4;
                        break;
diff --git usr.sbin/arp/arp.c usr.sbin/arp/arp.c
index b09eedd7ec9..09d5b5034b8 100644
--- usr.sbin/arp/arp.c
+++ usr.sbin/arp/arp.c
@@ -259,7 +259,6 @@ parse_host(const char *host, struct in_addr *in)
 struct sockaddr_in     so_mask = { 8, 0, 0, { 0xffffffff } };
 struct sockaddr_inarp  blank_sin = { sizeof(blank_sin), AF_INET }, sin_m;
 struct sockaddr_dl     blank_sdl = { sizeof(blank_sdl), AF_LINK }, sdl_m;
-struct sockaddr_dl     ifp_m = { sizeof(ifp_m), AF_LINK };
 time_t                 expire_time;
 int                    flags, export_only, doing_proxy, found_entry;
 struct {
@@ -646,7 +645,7 @@ rtmsg(int cmd)
                }
                /* FALLTHROUGH */
        case RTM_GET:
-               rtm->rtm_addrs |= (RTA_DST | RTA_IFP);
+               rtm->rtm_addrs |= RTA_DST;
        }
 
 #define NEXTADDR(w, s)                                                 \
@@ -658,7 +657,6 @@ rtmsg(int cmd)
        NEXTADDR(RTA_DST, sin_m);
        NEXTADDR(RTA_GATEWAY, sdl_m);
        NEXTADDR(RTA_NETMASK, so_mask);
-       NEXTADDR(RTA_IFP, ifp_m);
 
        rtm->rtm_msglen = cp - (char *)&m_rtmsg;
 doit:
-- 
2.26.2

Reply via email to