[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH net-next v4 06/15] net: skbuff: add skb_crc32c_full() that reports failure



skb_crc32c() has the same problem as skb_checksum().  When the range
runs into unreadable frags or past the end of the skb, skb_crc32c()
returns 0, which is also a valid CRC.  sctp_compute_cksum() and
skb_crc32c_csum_help() complement it, so a failed walk looks like a
checksum of 0xffffffff whatever the packet contains.

Add skb_crc32c_full(), which works like skb_checksum_full().
skb_crc32c() becomes a wrapper for the callers that aren't converted
yet.  The wrapper still returns 0 on failure, but it no longer warns
when only a frag_list member has unreadable frags.

Convert skb_crc32c_csum_help(), which can already return an error, so
that an SCTP packet that can't be checksummed is dropped on transmit.
The other callers will follow with the skb_checksum() callers.

Fixes: 20ddd0fd2fcd ("net: skbuff: don't BUG() on leftover length in 
skb_checksum() and friends")
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 include/linux/skbuff.h |   2 +
 net/core/dev.c         |   7 +++-
 net/core/skbuff.c      | 100 +++++++++++++++++++++++++++++++++++++------------
 3 files changed, 83 insertions(+), 26 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5a5354a6b6ba..5581ed3cbf47 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4332,6 +4332,8 @@ static inline int memcpy_to_msg(struct msghdr *msg, void 
*data, int len)
 
 __wsum skb_checksum(const struct sk_buff *skb, int offset, int len,
                    __wsum csum);
+int __must_check skb_crc32c_full(const struct sk_buff *skb, int offset,
+                                int len, u32 *crcp);
 u32 skb_crc32c(const struct sk_buff *skb, int offset, int len, u32 crc);
 
 static inline void * __must_check
diff --git a/net/core/dev.c b/net/core/dev.c
index 00a62c391056..f9878db781df 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3752,8 +3752,11 @@ int skb_crc32c_csum_help(struct sk_buff *skb)
        if (ret)
                goto out;
 
-       crc = ~skb_crc32c(skb, start, skb->len - start, ~0);
-       *(__le32 *)(skb->data + offset) = cpu_to_le32(crc);
+       crc = ~0;
+       ret = skb_crc32c_full(skb, start, skb->len - start, &crc);
+       if (ret)
+               goto out;
+       *(__le32 *)(skb->data + offset) = cpu_to_le32(~crc);
        skb_reset_csum_not_inet(skb);
 out:
        return ret;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 66ba9bfa8fc7..96c8835a262b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3825,23 +3825,79 @@ int skb_copy_and_csum_bits(const struct sk_buff *skb, 
int offset,
 EXPORT_SYMBOL(skb_copy_and_csum_bits);
 
 #ifdef CONFIG_NET_CRC32C
-u32 skb_crc32c(const struct sk_buff *skb, int offset, int len, u32 crc)
+/* The frag_list part of skb_crc32c_full().  It's out of line so that taking
+ * the address of crc doesn't cost every caller a stack protector.
+ */
+static noinline int skb_crc32c_frag_list(const struct sk_buff *skb, int start,
+                                        int offset, int len, u32 crc,
+                                        u32 *crcp)
+{
+       struct sk_buff *frag_iter;
+       int copy, err;
+
+       skb_walk_frags(skb, frag_iter) {
+               int end;
+
+               WARN_ON(start > offset + len);
+
+               end = start + frag_iter->len;
+               copy = end - offset;
+               if (copy > 0) {
+                       copy = min(copy, len);
+                       err = skb_crc32c_full(frag_iter, offset - start, copy,
+                                             &crc);
+                       if (err)
+                               return err;
+                       len -= copy;
+                       if (len == 0)
+                               goto out;
+                       offset += copy;
+               }
+               start = end;
+       }
+       if (unlikely(len)) {
+               DEBUG_NET_WARN_ON_ONCE(1);
+               return -EFAULT;
+       }
+out:
+       *crcp = crc;
+       return 0;
+}
+
+/**
+ *     skb_crc32c_full - CRC32C a range of an skb
+ *     @skb: buffer to read
+ *     @offset: offset in @skb to start at
+ *     @len: number of bytes to include
+ *     @crcp: the CRC to continue from; on success, updated with the range
+ *
+ *     Return: 0 on success, -EINVAL if @len is negative, or -EFAULT if @skb
+ *     runs out before @len does, or if the range goes past the linear area
+ *     of an skb, @skb or one on its frag_list, that has unreadable frags.
+ *     On failure *@crcp is not changed.
+ */
+int skb_crc32c_full(const struct sk_buff *skb, int offset, int len, u32 *crcp)
 {
        int start = skb_headlen(skb);
        int i, copy = start - offset;
-       struct sk_buff *frag_iter;
+       u32 crc = *crcp;
+
+       if (unlikely(len < 0)) {
+               DEBUG_NET_WARN_ON_ONCE(1);
+               return -EINVAL;
+       }
 
        if (copy > 0) {
                copy = min(copy, len);
                crc = crc32c(crc, skb->data + offset, copy);
                len -= copy;
                if (len == 0)
-                       return crc;
+                       goto out;
                offset += copy;
        }
 
-       if (WARN_ON_ONCE(!skb_frags_readable(skb)))
-               return 0;
+       if (!skb_frags_readable(skb))
+               return -EFAULT;
 
        for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
                int end;
@@ -3866,34 +3922,30 @@ u32 skb_crc32c(const struct sk_buff *skb, int offset, 
int len, u32 crc)
                        }
                        len -= copy;
                        if (len == 0)
-                               return crc;
+                               goto out;
                        offset += copy;
                }
                start = end;
        }
 
-       skb_walk_frags(skb, frag_iter) {
-               int end;
+       return skb_crc32c_frag_list(skb, start, offset, len, crc, crcp);
+out:
+       *crcp = crc;
+       return 0;
+}
+EXPORT_SYMBOL(skb_crc32c_full);
 
-               WARN_ON(start > offset + len);
+/* skb_crc32c() has no way to report failure, so it returns 0, which is a
+ * valid CRC.  Use skb_crc32c_full() in new code.
+ */
+u32 skb_crc32c(const struct sk_buff *skb, int offset, int len, u32 crc)
+{
+       int err = skb_crc32c_full(skb, offset, len, &crc);
 
-               end = start + frag_iter->len;
-               copy = end - offset;
-               if (copy > 0) {
-                       copy = min(copy, len);
-                       crc = skb_crc32c(frag_iter, offset - start, copy, crc);
-                       len -= copy;
-                       if (len == 0)
-                               return crc;
-                       offset += copy;
-               }
-               start = end;
-       }
-       if (unlikely(len)) {
-               DEBUG_NET_WARN_ON_ONCE(1);
+       if (unlikely(err)) {
+               WARN_ON_ONCE(err == -EFAULT && !skb_frags_readable(skb));
                return 0;
        }
-
        return crc;
 }
 EXPORT_SYMBOL(skb_crc32c);

-- 
2.55.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.