netdev - Re: route: an issue caused by local and main table's merge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKgT0UcbycqgrfviqUmvS9S7+F6q-gMzrz-KKQuEb77ruZZLRQ@mail.gmail.com>
Date:   Mon, 9 Mar 2020 08:53:53 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     David Ahern <dsahern@...il.com>
Cc:     Xin Long <lucien.xin@...il.com>,
        network dev <netdev@...r.kernel.org>,
        davem <davem@...emloft.net>, mmhatre@...hat.com,
        "alexander.h.duyck@...el.com" <alexander.h.duyck@...el.com>
Subject: Re: route: an issue caused by local and main table's merge

On Sun, Mar 8, 2020 at 7:31 PM David Ahern <dsahern@...il.com> wrote:
>
> [ This got lost in the backlog ]
>
> On 3/2/20 1:38 AM, Xin Long wrote:
> > Hi, David A.
> >
> > Mithil reported an issue, which can be reproduced by:
> >
> >   # ip link  add dummy0 type dummy
> >   # ip link  set dummy0 up
> >   # ip route add to broadcast 192.168.122.1 dev dummy0 <--- broadcast
> >   # ip route add 192.168.122.1 dev dummy0   <--- unicast
> >   # ip route add 1.1.1.1 via 192.168.122.1  <--- [A]
> >   Error: Nexthop has invalid gateway.
> >   # ip rule  add from 2.2.2.2
> >   # ip route add 1.1.1.1 via 192.168.122.1  <--- [B]
> >
> > cmd [A] failed , as in fib_check_nh_v4_gw():
> >
> >     if (table)
> >             tbl = fib_get_table(net, table);
> >
> >     if (tbl)
> >             err = fib_table_lookup_2(tbl, &fl4, &res,
> >                                    FIB_LOOKUP_IGNORE_LINKSTATE |
> >                                    FIB_LOOKUP_NOREF);
> >
> >     if (res.type != RTN_UNICAST && res.type != RTN_LOCAL) { <--- [a]
> >             NL_SET_ERR_MSG(extack, "Nexthop has invalid gateway");
> >             goto out;  <--[a]
> >     }
> >
> > It gets the route for '192.168.122.1' from the merged (main/local)
> > table, and the broadcast one returns, and it fails the check [a].
> >
> > But the same cmd [B] will work after one rule is added, by which
> > main table and local table get separated, it gets the route from
> > the main table (the same table for this route), and the unicast
> > one returns, and it will pass the check [a].
> >
> > Any idea on how to fix this, and keep it consistent before and
> > after a rule added?
> >
>
> I do not have any suggestions off the top of my head.
>
> Adding Alex who as I recall did the table merge.

As far as the table merge it is undone as soon as any rules are added
or deleted. From that point on it will not re-merge the table. So if
you are using rules you are going to need to modify the rules first
and then add routes as this will guarantee the table is consistent as
any further rule modifications will not re-merge the table.

Also, is it really a valid configuration to have the same address
configured as both a broadcast and unicast address? I couldn't find
anything that said it wasn't, but at the same time I haven't found
anything saying it is an acceptable practice to configure an IP
address as both a broadcast and unicast destination. Everything I saw
seemed to imply that a subnet should be at least a /30 to guarantee a
pair of IPs and support for broadcast addresses with all 1's and 0 for
the host identifier. As such 192.168.122.1 would never really be a
valid broadcast address since it implies a /31 subnet mask.

- Alex