| Message ID | f6e832e7632acf28b1d2b35dddb08769c7ce4fab.1595624517.git.gnault@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] bareudp: forbid mixing IP and MPLS in multiproto mode | expand |
On Fri, 24 Jul 2020 23:03:26 +0200 Guillaume Nault wrote: > In multiproto mode, bareudp_xmit() accepts sending multicast MPLS and > IPv6 packets regardless of the bareudp ethertype. In practice, this > let an IP tunnel send multicast MPLS packets, or an MPLS tunnel send > IPv6 packets. > > We need to restrict the test further, so that the multiproto mode only > enables > * IPv6 for IPv4 tunnels, > * or multicast MPLS for unicast MPLS tunnels. > > To improve clarity, the protocol validation is moved to its own > function, where each logical test has its own condition. > > Fixes: 4b5f67232d95 ("net: Special handling for IP & MPLS.") > Signed-off-by: Guillaume Nault <gnault@redhat.com> Hi! this adds 10 sparse warnings: drivers/net/bareudp.c:419:22: warning: cast to restricted __be16 drivers/net/bareudp.c:419:22: warning: cast to restricted __be16 drivers/net/bareudp.c:419:22: warning: cast to restricted __be16 drivers/net/bareudp.c:419:22: warning: cast to restricted __be16 drivers/net/bareudp.c:419:13: warning: restricted __be16 degrades to integer drivers/net/bareudp.c:423:22: warning: cast to restricted __be16 drivers/net/bareudp.c:423:22: warning: cast to restricted __be16 drivers/net/bareudp.c:423:22: warning: cast to restricted __be16 drivers/net/bareudp.c:423:22: warning: cast to restricted __be16 I think this: proto == ntohs(ETH_P_MPLS_MC)) has to say htons() not ntohs(). For v6 as well.
On Fri, Jul 24, 2020 at 04:21:34PM -0700, Jakub Kicinski wrote: > On Fri, 24 Jul 2020 23:03:26 +0200 Guillaume Nault wrote: > > In multiproto mode, bareudp_xmit() accepts sending multicast MPLS and > > IPv6 packets regardless of the bareudp ethertype. In practice, this > > let an IP tunnel send multicast MPLS packets, or an MPLS tunnel send > > IPv6 packets. > > > > We need to restrict the test further, so that the multiproto mode only > > enables > > * IPv6 for IPv4 tunnels, > > * or multicast MPLS for unicast MPLS tunnels. > > > > To improve clarity, the protocol validation is moved to its own > > function, where each logical test has its own condition. > > > > Fixes: 4b5f67232d95 ("net: Special handling for IP & MPLS.") > > Signed-off-by: Guillaume Nault <gnault@redhat.com> > > Hi! this adds 10 sparse warnings: > > drivers/net/bareudp.c:419:22: warning: cast to restricted __be16 > drivers/net/bareudp.c:419:22: warning: cast to restricted __be16 > drivers/net/bareudp.c:419:22: warning: cast to restricted __be16 > drivers/net/bareudp.c:419:22: warning: cast to restricted __be16 > drivers/net/bareudp.c:419:13: warning: restricted __be16 degrades to integer > drivers/net/bareudp.c:423:22: warning: cast to restricted __be16 > drivers/net/bareudp.c:423:22: warning: cast to restricted __be16 > drivers/net/bareudp.c:423:22: warning: cast to restricted __be16 > drivers/net/bareudp.c:423:22: warning: cast to restricted __be16 > > I think this: > > proto == ntohs(ETH_P_MPLS_MC)) > > has to say htons() not ntohs(). For v6 as well. > Ouch, sorry. I'll respin.
diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c index 3dd46cd55114..e97f318f9f06 100644 --- a/drivers/net/bareudp.c +++ b/drivers/net/bareudp.c @@ -407,19 +407,34 @@ static int bareudp6_xmit_skb(struct sk_buff *skb, struct net_device *dev, return err; } +static bool bareudp_proto_valid(struct bareudp_dev *bareudp, __be16 proto) +{ + if (bareudp->ethertype == proto) + return true; + + if (!bareudp->multi_proto_mode) + return false; + + if (bareudp->ethertype == htons(ETH_P_MPLS_UC) && + proto == ntohs(ETH_P_MPLS_MC)) + return true; + + if (bareudp->ethertype == htons(ETH_P_IP) && + proto == ntohs(ETH_P_IPV6)) + return true; + + return false; +} + static netdev_tx_t bareudp_xmit(struct sk_buff *skb, struct net_device *dev) { struct bareudp_dev *bareudp = netdev_priv(dev); struct ip_tunnel_info *info = NULL; int err; - if (skb->protocol != bareudp->ethertype) { - if (!bareudp->multi_proto_mode || - (skb->protocol != htons(ETH_P_MPLS_MC) && - skb->protocol != htons(ETH_P_IPV6))) { - err = -EINVAL; - goto tx_error; - } + if (!bareudp_proto_valid(bareudp, skb->protocol)) { + err = -EINVAL; + goto tx_error; } info = skb_tunnel_info(skb);
In multiproto mode, bareudp_xmit() accepts sending multicast MPLS and IPv6 packets regardless of the bareudp ethertype. In practice, this let an IP tunnel send multicast MPLS packets, or an MPLS tunnel send IPv6 packets. We need to restrict the test further, so that the multiproto mode only enables * IPv6 for IPv4 tunnels, * or multicast MPLS for unicast MPLS tunnels. To improve clarity, the protocol validation is moved to its own function, where each logical test has its own condition. Fixes: 4b5f67232d95 ("net: Special handling for IP & MPLS.") Signed-off-by: Guillaume Nault <gnault@redhat.com> --- drivers/net/bareudp.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-)