All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] bareudp: forbid mixing IP and MPLS in multiproto mode
@ 2020-07-24 21:03 Guillaume Nault
  2020-07-24 23:21 ` Jakub Kicinski
  0 siblings, 1 reply; 3+ messages in thread
From: Guillaume Nault @ 2020-07-24 21:03 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: netdev, Martin Varghese, Willem de Bruijn

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(-)

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);
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] bareudp: forbid mixing IP and MPLS in multiproto mode
  2020-07-24 21:03 [PATCH net] bareudp: forbid mixing IP and MPLS in multiproto mode Guillaume Nault
@ 2020-07-24 23:21 ` Jakub Kicinski
  2020-07-25 11:05   ` Guillaume Nault
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2020-07-24 23:21 UTC (permalink / raw)
  To: Guillaume Nault; +Cc: David Miller, netdev, Martin Varghese, Willem de Bruijn

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] bareudp: forbid mixing IP and MPLS in multiproto mode
  2020-07-24 23:21 ` Jakub Kicinski
@ 2020-07-25 11:05   ` Guillaume Nault
  0 siblings, 0 replies; 3+ messages in thread
From: Guillaume Nault @ 2020-07-25 11:05 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: David Miller, netdev, Martin Varghese, Willem de Bruijn

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.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-07-25 11:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-24 21:03 [PATCH net] bareudp: forbid mixing IP and MPLS in multiproto mode Guillaume Nault
2020-07-24 23:21 ` Jakub Kicinski
2020-07-25 11:05   ` Guillaume Nault

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.