v3: linearize based on layer4 protocol.
v4: no WARN_ON_ONCE call

-- 8< --
This patch fixes a silent out-of-bound read possibility that was present
because of the misuse of this function.

Mostly it was called with a struct udphdr *hp which had only the udphdr
part linearized by the skb_header_pointer, however
nf_tproxy_get_sock_v{4,6} uses it as a tcphdr pointer, so some reads for
tcp specific attributes may be invalid.

Fixes: a583636a83ea ("inet: refactor inet[6]_lookup functions to take skb")
Signed-off-by: Máté Eckl <[email protected]>
---
 include/net/netfilter/nf_tproxy.h   |  4 ++--
 net/ipv4/netfilter/nf_tproxy_ipv4.c | 16 +++++++++++-----
 net/ipv6/netfilter/nf_tproxy_ipv6.c | 16 +++++++++++-----
 net/netfilter/xt_TPROXY.c           |  8 ++++----
 4 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/include/net/netfilter/nf_tproxy.h 
b/include/net/netfilter/nf_tproxy.h
index 9754a50ecde9..4cc64c8446eb 100644
--- a/include/net/netfilter/nf_tproxy.h
+++ b/include/net/netfilter/nf_tproxy.h
@@ -64,7 +64,7 @@ nf_tproxy_handle_time_wait4(struct net *net, struct sk_buff 
*skb,
  * belonging to established connections going through that one.
  */
 struct sock *
-nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb, void *hp,
+nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb,
                      const u8 protocol,
                      const __be32 saddr, const __be32 daddr,
                      const __be16 sport, const __be16 dport,
@@ -103,7 +103,7 @@ nf_tproxy_handle_time_wait6(struct sk_buff *skb, int 
tproto, int thoff,
                            struct sock *sk);
 
 struct sock *
-nf_tproxy_get_sock_v6(struct net *net, struct sk_buff *skb, int thoff, void 
*hp,
+nf_tproxy_get_sock_v6(struct net *net, struct sk_buff *skb, int thoff,
                      const u8 protocol,
                      const struct in6_addr *saddr, const struct in6_addr 
*daddr,
                      const __be16 sport, const __be16 dport,
diff --git a/net/ipv4/netfilter/nf_tproxy_ipv4.c 
b/net/ipv4/netfilter/nf_tproxy_ipv4.c
index 805e83ec3ad9..e2559a1cdbf4 100644
--- a/net/ipv4/netfilter/nf_tproxy_ipv4.c
+++ b/net/ipv4/netfilter/nf_tproxy_ipv4.c
@@ -37,7 +37,7 @@ nf_tproxy_handle_time_wait4(struct net *net, struct sk_buff 
*skb,
                 * to a listener socket if there's one */
                struct sock *sk2;
 
-               sk2 = nf_tproxy_get_sock_v4(net, skb, hp, iph->protocol,
+               sk2 = nf_tproxy_get_sock_v4(net, skb, iph->protocol,
                                            iph->saddr, laddr ? laddr : 
iph->daddr,
                                            hp->source, lport ? lport : 
hp->dest,
                                            skb->dev, 
NF_TPROXY_LOOKUP_LISTENER);
@@ -71,24 +71,27 @@ __be32 nf_tproxy_laddr4(struct sk_buff *skb, __be32 
user_laddr, __be32 daddr)
 EXPORT_SYMBOL_GPL(nf_tproxy_laddr4);
 
 struct sock *
-nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb, void *hp,
+nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb,
                      const u8 protocol,
                      const __be32 saddr, const __be32 daddr,
                      const __be16 sport, const __be16 dport,
                      const struct net_device *in,
                      const enum nf_tproxy_lookup_t lookup_type)
 {
+       struct tcphdr _hdr, *hp;
        struct sock *sk;
-       struct tcphdr *tcph;
 
        switch (protocol) {
        case IPPROTO_TCP:
+               hp = skb_header_pointer(skb, ip_hdrlen(skb),
+                                       sizeof(struct tcphdr), &_hdr);
+               if (hp == NULL)
+                       return NULL;
                switch (lookup_type) {
                case NF_TPROXY_LOOKUP_LISTENER:
-                       tcph = hp;
                        sk = inet_lookup_listener(net, &tcp_hashinfo, skb,
                                                    ip_hdrlen(skb) +
-                                                     __tcp_hdrlen(tcph),
+                                                     __tcp_hdrlen(hp),
                                                    saddr, sport,
                                                    daddr, dport,
                                                    in->ifindex, 0);
@@ -111,6 +114,9 @@ nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb, 
void *hp,
                }
                break;
        case IPPROTO_UDP:
+               /* hp and _hdr is not used here so skb_header_pointer do not
+                * need to be called
+                */
                sk = udp4_lib_lookup(net, saddr, sport, daddr, dport,
                                     in->ifindex);
                if (sk) {
diff --git a/net/ipv6/netfilter/nf_tproxy_ipv6.c 
b/net/ipv6/netfilter/nf_tproxy_ipv6.c
index bf1d6c421e3b..3d4b087a846f 100644
--- a/net/ipv6/netfilter/nf_tproxy_ipv6.c
+++ b/net/ipv6/netfilter/nf_tproxy_ipv6.c
@@ -55,7 +55,7 @@ nf_tproxy_handle_time_wait6(struct sk_buff *skb, int tproto, 
int thoff,
                 * to a listener socket if there's one */
                struct sock *sk2;
 
-               sk2 = nf_tproxy_get_sock_v6(net, skb, thoff, hp, tproto,
+               sk2 = nf_tproxy_get_sock_v6(net, skb, thoff, tproto,
                                            &iph->saddr,
                                            nf_tproxy_laddr6(skb, laddr, 
&iph->daddr),
                                            hp->source,
@@ -72,23 +72,26 @@ nf_tproxy_handle_time_wait6(struct sk_buff *skb, int 
tproto, int thoff,
 EXPORT_SYMBOL_GPL(nf_tproxy_handle_time_wait6);
 
 struct sock *
-nf_tproxy_get_sock_v6(struct net *net, struct sk_buff *skb, int thoff, void 
*hp,
+nf_tproxy_get_sock_v6(struct net *net, struct sk_buff *skb, int thoff,
                      const u8 protocol,
                      const struct in6_addr *saddr, const struct in6_addr 
*daddr,
                      const __be16 sport, const __be16 dport,
                      const struct net_device *in,
                      const enum nf_tproxy_lookup_t lookup_type)
 {
+       struct tcphdr _hdr, *hp;
        struct sock *sk;
-       struct tcphdr *tcph;
 
        switch (protocol) {
        case IPPROTO_TCP:
+               hp = skb_header_pointer(skb, thoff,
+                                       sizeof(struct tcphdr), &_hdr);
+               if (hp == NULL)
+                       return NULL;
                switch (lookup_type) {
                case NF_TPROXY_LOOKUP_LISTENER:
-                       tcph = hp;
                        sk = inet6_lookup_listener(net, &tcp_hashinfo, skb,
-                                                  thoff + __tcp_hdrlen(tcph),
+                                                  thoff + __tcp_hdrlen(hp),
                                                   saddr, sport,
                                                   daddr, ntohs(dport),
                                                   in->ifindex, 0);
@@ -111,6 +114,9 @@ nf_tproxy_get_sock_v6(struct net *net, struct sk_buff *skb, 
int thoff, void *hp,
                }
                break;
        case IPPROTO_UDP:
+               /* hp and _hdr is not used here so skb_header_pointer do not
+                * need to be called
+                */
                sk = udp6_lib_lookup(net, saddr, sport, daddr, dport,
                                     in->ifindex);
                if (sk) {
diff --git a/net/netfilter/xt_TPROXY.c b/net/netfilter/xt_TPROXY.c
index 58fce4e749a9..d76550a8b642 100644
--- a/net/netfilter/xt_TPROXY.c
+++ b/net/netfilter/xt_TPROXY.c
@@ -61,7 +61,7 @@ tproxy_tg4(struct net *net, struct sk_buff *skb, __be32 
laddr, __be16 lport,
         * addresses, this happens if the redirect already happened
         * and the current packet belongs to an already established
         * connection */
-       sk = nf_tproxy_get_sock_v4(net, skb, hp, iph->protocol,
+       sk = nf_tproxy_get_sock_v4(net, skb, iph->protocol,
                                   iph->saddr, iph->daddr,
                                   hp->source, hp->dest,
                                   skb->dev, NF_TPROXY_LOOKUP_ESTABLISHED);
@@ -77,7 +77,7 @@ tproxy_tg4(struct net *net, struct sk_buff *skb, __be32 
laddr, __be16 lport,
        else if (!sk)
                /* no, there's no established connection, check if
                 * there's a listener on the redirected addr/port */
-               sk = nf_tproxy_get_sock_v4(net, skb, hp, iph->protocol,
+               sk = nf_tproxy_get_sock_v4(net, skb, iph->protocol,
                                           iph->saddr, laddr,
                                           hp->source, lport,
                                           skb->dev, NF_TPROXY_LOOKUP_LISTENER);
@@ -150,7 +150,7 @@ tproxy_tg6_v1(struct sk_buff *skb, const struct 
xt_action_param *par)
         * addresses, this happens if the redirect already happened
         * and the current packet belongs to an already established
         * connection */
-       sk = nf_tproxy_get_sock_v6(xt_net(par), skb, thoff, hp, tproto,
+       sk = nf_tproxy_get_sock_v6(xt_net(par), skb, thoff, tproto,
                                   &iph->saddr, &iph->daddr,
                                   hp->source, hp->dest,
                                   xt_in(par), NF_TPROXY_LOOKUP_ESTABLISHED);
@@ -171,7 +171,7 @@ tproxy_tg6_v1(struct sk_buff *skb, const struct 
xt_action_param *par)
        else if (!sk)
                /* no there's no established connection, check if
                 * there's a listener on the redirected addr/port */
-               sk = nf_tproxy_get_sock_v6(xt_net(par), skb, thoff, hp,
+               sk = nf_tproxy_get_sock_v6(xt_net(par), skb, thoff,
                                           tproto, &iph->saddr, laddr,
                                           hp->source, lport,
                                           xt_in(par), 
NF_TPROXY_LOOKUP_LISTENER);
-- 
ecklm

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to