rtsock: Check prefixlength before deciding an existing L2 cached route is the same as a new route being added

[Mitchell Krome]

Hi,

I ran into a problem where when using /31 netmasks on point to point
links, I am unable to add a larger summary route that happens to have
the same network address as the /31 link route the box has. It's a
bit hard to explain so hopefully the art below helps.


                bsd1                                rtr2
      +---------------------+             +---------------------+
      | vio0  172.19.6.33/31|<----------->|172.19.6.32/31  if1  |
      +---------------------+             +---------------------+
172.19.6.32/27 -> 172.19.6.32

$route show
...
172.17.6.32/31     172.17.6.33        UCn        1        0     -     4 vio0
172.17.6.32        50:40:61:d6:05:e3  UHLch     79     5959     -     3 vio0
172.17.6.33        52:54:00:e7:0c:c5  UHLl       0       70     -     1 vio0

When I try to add the /27 route, it comes back with invalid argument

$ doas route add 172.17.6.32/27 172.17.6.32
add net 172.17.6.32/27: gateway 172.17.6.32: Invalid argument

I managed to track this down to a check in rtsock.c where it thinks that
the route I am adding is a modification of the existing L2 route for
172.19.7.32. The Invalid Argument is because later on in the RTM_CHANGE
code it wants a AF of the new nexthop to be the same as the route it
thinks its changing. When using /30 routes this would never happen as
you wouldn't get a host on the network address I guess. 

Diff below fixed the problem by also checking the prefix length matches
before assuming the route isn't a new route and jumping into RTM_CHANGE
code.

The code in change: also checks the AF are the same, before it will
do a modification so I'm not sure if this should also check that before
assuming the route isn't actually a new one?

As far as I could tell reading the other code, it seemed like the only
way to get RTF_CACHED flag was to be a L2 nexthop, so I don't think that
there would be a case where the prefix length of the new and old entries
don't match but the intent was to go through the change path. 




diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c
index a717d112e..9dbc8ca90 100644
--- a/sys/net/rtsock.c
+++ b/sys/net/rtsock.c
@@ -937,7 +937,10 @@ rtm_output(struct rt_msghdr *rtm, struct rtentry **prt,
                 * cached route because this can lead to races in the
                 * receive path.  Instead we update the L2 cache.
                 */
-               if ((rt != NULL) && ISSET(rt->rt_flags, RTF_CACHED)) {
+               plen = rtable_satoplen(info->rti_info[RTAX_DST]->sa_family,
+                   info->rti_info[RTAX_NETMASK]);
+               if ((rt != NULL) && (plen == rt->rt_plen) &&
+                   ISSET(rt->rt_flags, RTF_CACHED)) {
                        ifp = if_get(rt->rt_ifidx);
                        if (ifp == NULL) {
                                rtfree(rt);