lwt: Check LWTUNNEL_XMIT_CONTINUE strictly

LWTUNNEL_XMIT_CONTINUE is implicitly assumed in ip(6)_finish_output2,
such that any positive return value from a xmit hook could cause
unexpected continue behavior, despite that related skb may have been
freed. This could be error-prone for future xmit hook ops. One of the
possible errors is to return statuses of dst_output directly.

To make the code safer, redefine LWTUNNEL_XMIT_CONTINUE value to
distinguish from dst_output statuses and check the continue
condition explicitly.

Fixes: 3a0af8fd61f9 ("bpf: BPF for lightweight tunnel infrastructure")
Suggested-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Yan Zhai <yan@cloudflare.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/96b939b85eda00e8df4f7c080f770970a4c5f698.1692326837.git.yan@cloudflare.com

diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index 6f15e6fa154e6ba8343ed37f87218256f8dfe6ab..53bd2d02a4f0db374b3920386afd12b0d7cbe6a0 100644 (file)
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -16,9 +16,12 @@
 #define LWTUNNEL_STATE_INPUT_REDIRECT  BIT(1)
 #define LWTUNNEL_STATE_XMIT_REDIRECT   BIT(2)
 
+/* LWTUNNEL_XMIT_CONTINUE should be distinguishable from dst_output return
+ * values (NET_XMIT_xxx and NETDEV_TX_xxx in linux/netdevice.h) for safety.
+ */
 enum {
        LWTUNNEL_XMIT_DONE,
-       LWTUNNEL_XMIT_CONTINUE,
+       LWTUNNEL_XMIT_CONTINUE = 0x100,
 };
 
 

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index ce6257860a4019d01e28d57d3ce4981fe79d0a0e..43ba4b77b2484a38031a919c9be29eba9af3f031 100644 (file)
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -216,7 +216,7 @@ static int ip_finish_output2(struct net *net, struct sock *sk, struct sk_buff *s
        if (lwtunnel_xmit_redirect(dst->lwtstate)) {
                int res = lwtunnel_xmit(skb);
 
-               if (res < 0 || res == LWTUNNEL_XMIT_DONE)
+               if (res != LWTUNNEL_XMIT_CONTINUE)
                        return res;
        }
 

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index f8a1f6bb3f87251836fe6a478f16ef948239ed93..0665e8b099680c9bea605da25b06a3d03ced1c78 100644 (file)
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -113,7 +113,7 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
        if (lwtunnel_xmit_redirect(dst->lwtstate)) {
                int res = lwtunnel_xmit(skb);
 
-               if (res < 0 || res == LWTUNNEL_XMIT_DONE)
+               if (res != LWTUNNEL_XMIT_CONTINUE)
                        return res;
        }