netfilter: handle unreadable frags

sashiko reports:
 When an skb with unreadable fragments (such as from devmem TCP, where
 skb_frags_readable(skb) returns false) is processed by the u32 module,
 skb_copy_bits() will safely return a negative error code [..]

xt_u32: bail out with hotdrop in this case.
gather_frags: return -1, just as if we had no fragment header.
nfnetlink_queue: restrict to the linear part.
nfnetlink_log: restrict to the linear part.

v2:
 - skb_zerocopy helpers don't copy readable flag, i.e. nfnetlink_queue
 is broken too
 xt_u32 shouldn't return true if hotdrop was set.

Fixes: 65249feb6b3d ("net: add support for skbs with unreadable frags")
Cc: stable@vger.kernel.org
Acked-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>

diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 3637b20d3fa44e5ff92c56082d05be3374824e76..599c49bf0a0afd5054e926a5f2b1f0677bcd8b4d 100644 (file)
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -419,7 +419,7 @@ find_prev_fhdr(struct sk_buff *skb, u8 *prevhdrp, int *prevhoff, int *fhoff)
                        return -1;
                }
                if (skb_copy_bits(skb, start, &hdr, sizeof(hdr)))
-                       BUG();
+                       return -1;
                if (nexthdr == NEXTHDR_AUTH)
                        hdrlen = ipv6_authlen(&hdr);
                else

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index fa36575998616f63798b819cd84961493010aa48..5fee61b3813cbc146ce98e9cc0bfa24b115768e5 100644 (file)
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -676,7 +676,7 @@ __build_packet_message(struct nfnl_log_net *log,
                        goto nla_put_failure;
 
                if (skb_copy_bits(skb, 0, nla_data(nla), data_len))
-                       BUG();
+                       goto nla_put_failure;
        }
 
        nlh->nlmsg_len = inst->skb->tail - old_tail;
@@ -698,6 +698,21 @@ static const struct nf_loginfo default_loginfo = {
        },
 };
 
+static unsigned int nfulnl_get_copy_len(const struct nf_loginfo *li,
+                                       const struct sk_buff *skb,
+                                       unsigned int copy_len)
+{
+       unsigned int len = skb->len;
+
+       if ((li->u.ulog.flags & NF_LOG_F_COPY_LEN) &&
+           li->u.ulog.copy_len < copy_len)
+               copy_len = li->u.ulog.copy_len;
+       if (!skb_frags_readable(skb))
+               len = skb_headlen(skb);
+
+       return min(len, copy_len);
+}
+
 /* log handler for internal netfilter logging api */
 static void
 nfulnl_log_packet(struct net *net,
@@ -790,14 +805,7 @@ nfulnl_log_packet(struct net *net,
                break;
 
        case NFULNL_COPY_PACKET:
-               data_len = inst->copy_range;
-               if ((li->u.ulog.flags & NF_LOG_F_COPY_LEN) &&
-                   (li->u.ulog.copy_len < data_len))
-                       data_len = li->u.ulog.copy_len;
-
-               if (data_len > skb->len)
-                       data_len = skb->len;
-
+               data_len = nfulnl_get_copy_len(li, skb, inst->copy_range);
                size += nla_total_size(data_len);
                break;
 

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 35d4c6c628ffc19cee9e44584bd37514a82a0835..b8aaf39cb4d8eafb27c0200b51fb4472aad1c272 100644 (file)
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -690,6 +690,17 @@ static int nfqnl_put_master_ifindex(struct sk_buff *nlskb, int attr,
 }
 #endif
 
+static unsigned int nfqnl_get_data_len(const struct sk_buff *entskb,
+                                      unsigned int copy_range)
+{
+       unsigned int data_len = entskb->len;
+
+       if (!skb_frags_readable(entskb))
+               data_len = skb_headlen(entskb);
+
+       return min(data_len, copy_range);
+}
+
 static struct sk_buff *
 nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
                           struct nf_queue_entry *entry,
@@ -755,10 +766,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
                    nf_queue_checksum_help(entskb))
                        return NULL;
 
-               data_len = READ_ONCE(queue->copy_range);
-               if (data_len > entskb->len)
-                       data_len = entskb->len;
-
+               data_len = nfqnl_get_data_len(entskb, READ_ONCE(queue->copy_range));
                hlen = skb_zerocopy_headlen(entskb);
                hlen = min_t(unsigned int, hlen, data_len);
                size += sizeof(struct nlattr) + hlen;

diff --git a/net/netfilter/xt_u32.c b/net/netfilter/xt_u32.c
index ec1a21e3b6e2c3f9d6c4f7f325d23648d8a47404..dabbaa742874cb91737b2713bbba4ead69f31c4d 100644 (file)
--- a/net/netfilter/xt_u32.c
+++ b/net/netfilter/xt_u32.c
@@ -14,8 +14,8 @@
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_u32.h>
 
-static bool u32_match_it(const struct xt_u32 *data,
-                        const struct sk_buff *skb)
+static int u32_match_it(const struct xt_u32 *data,
+                       const struct sk_buff *skb)
 {
        const struct xt_u32_test *ct;
        unsigned int testind;
@@ -40,7 +40,8 @@ static bool u32_match_it(const struct xt_u32 *data,
                        return false;
 
                if (skb_copy_bits(skb, pos, &n, sizeof(n)) < 0)
-                       BUG();
+                       return -1;
+
                val   = ntohl(n);
                nnums = ct->nnums;
 
@@ -68,7 +69,7 @@ static bool u32_match_it(const struct xt_u32 *data,
 
                                if (skb_copy_bits(skb, at + pos, &n,
                                                    sizeof(n)) < 0)
-                                       BUG();
+                                       return -1;
                                val = ntohl(n);
                                break;
                        }
@@ -90,9 +91,14 @@ static bool u32_match_it(const struct xt_u32 *data,
 static bool u32_mt(const struct sk_buff *skb, struct xt_action_param *par)
 {
        const struct xt_u32 *data = par->matchinfo;
-       bool ret;
+       int ret;
 
        ret = u32_match_it(data, skb);
+       if (ret < 0) {
+               par->hotdrop = true;
+               return false;
+       }
+
        return ret ^ data->invert;
 }