skb_checksum() can't fail. When the range runs into unreadable frags or past the end of the skb, skb_checksum() returns 0, which is also a valid checksum, so its callers can't tell a sum from a failure. Add skb_checksum_full(), which adds the range's sum to *@csump and returns 0. On failure it leaves *@csump alone and returns -EFAULT, or -EINVAL for a negative length. It is __must_check. Its frag_list loop sits in a noinline function so that taking the address of a local doesn't give skb_checksum_full() a stack protector. skb_checksum() becomes a wrapper for the callers that aren't converted yet, and still returns 0 on failure. Three things change for them: - When a frag_list member fails, skb_checksum() returns 0 for the whole range instead of the sum of the parts it could read. Neither value is the checksum of the range. - skb_checksum() only warns about unreadable frags if @skb itself has them. - A negative length returns 0 instead of reaching csum_partial(). Convert skb_checksum_help(), which can already return an error, so that a packet that can't be checksummed is dropped instead of sent with a wrong checksum. The following patches convert checksum validation, skb_segment() and pskb_trim_rcsum(). The rest will follow per subsystem. Fixes: 20ddd0fd2fcd ("net: skbuff: don't BUG() on leftover length in skb_checksum() and friends") Signed-off-by: Josef Bacik --- include/linux/skbuff.h | 2 + net/core/dev.c | 7 +++- net/core/skbuff.c | 111 ++++++++++++++++++++++++++++++++++++------------- 3 files changed, 90 insertions(+), 30 deletions(-) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index f4c092b78006..608c1491597e 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -4282,6 +4282,8 @@ int skb_copy_bits(const struct sk_buff *skb, int offset, void *to, int len); int skb_store_bits(struct sk_buff *skb, int offset, const void *from, int len); int __must_check skb_copy_and_csum_bits(const struct sk_buff *skb, int offset, u8 *to, int len, __wsum *csump); +int __must_check skb_checksum_full(const struct sk_buff *skb, int offset, + int len, __wsum *csump); int skb_splice_bits(struct sk_buff *skb, struct sock *sk, unsigned int offset, struct pipe_inode_info *pipe, unsigned int len, unsigned int flags); diff --git a/net/core/dev.c b/net/core/dev.c index 3ac700d37e59..00a62c391056 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3665,7 +3665,7 @@ void skb_warn_bad_offload(const struct sk_buff *skb) */ int skb_checksum_help(struct sk_buff *skb) { - __wsum csum; + __wsum csum = 0; int ret = 0, offset; if (skb->ip_summed == CHECKSUM_COMPLETE) @@ -3697,13 +3697,16 @@ int skb_checksum_help(struct sk_buff *skb) offset, skb_headlen(skb)); goto out; } - csum = skb_checksum(skb, offset, skb->len - offset, 0); + ret = skb_checksum_full(skb, offset, skb->len - offset, &csum); + if (ret) + goto out; offset += skb->csum_offset; if (unlikely(offset + sizeof(__sum16) > skb_headlen(skb))) { DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false); WARN_ONCE(true, "offset+2 (%zu) > skb_headlen() (%u)\n", offset + sizeof(__sum16), skb_headlen(skb)); + ret = -EINVAL; goto out; } ret = skb_ensure_writable(skb, offset + sizeof(__sum16)); diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 4ac2b476b0a9..c5f8aca89221 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -3536,27 +3536,88 @@ int skb_store_bits(struct sk_buff *skb, int offset, const void *from, int len) } EXPORT_SYMBOL(skb_store_bits); -/* Checksum skb data. */ -__wsum skb_checksum(const struct sk_buff *skb, int offset, int len, __wsum csum) +/* The frag_list part of skb_checksum_full(). It's out of line so that taking + * the address of csum2 doesn't cost every caller a stack protector. + */ +static noinline int skb_checksum_frag_list(const struct sk_buff *skb, int start, + int offset, int len, int pos, + __wsum csum, __wsum *csump) +{ + 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) { + __wsum csum2 = 0; + + if (copy > len) + copy = len; + err = skb_checksum_full(frag_iter, offset - start, copy, + &csum2); + if (err) + return err; + csum = csum_block_add(csum, csum2, pos); + len -= copy; + if (len == 0) + goto out; + offset += copy; + pos += copy; + } + start = end; + } + if (unlikely(len)) { + DEBUG_NET_WARN_ON_ONCE(1); + return -EFAULT; + } +out: + *csump = csum; + return 0; +} + +/** + * skb_checksum_full - checksum a range of an skb + * @skb: buffer to checksum + * @offset: offset in @skb to start at + * @len: number of bytes to checksum + * @csump: the sum to add to; on success the range's sum is added to it + * + * 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 *@csump is not changed. + */ +int skb_checksum_full(const struct sk_buff *skb, int offset, int len, + __wsum *csump) { int start = skb_headlen(skb); int i, copy = start - offset; - struct sk_buff *frag_iter; + __wsum csum = *csump; int pos = 0; + if (unlikely(len < 0)) { + DEBUG_NET_WARN_ON_ONCE(1); + return -EINVAL; + } + /* Checksum header. */ if (copy > 0) { if (copy > len) copy = len; csum = csum_partial(skb->data + offset, copy, csum); if ((len -= copy) == 0) - return csum; + goto out; offset += copy; pos = 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; @@ -3585,37 +3646,31 @@ __wsum skb_checksum(const struct sk_buff *skb, int offset, int len, __wsum csum) } if (!(len -= copy)) - return csum; + goto out; offset += copy; } start = end; } - skb_walk_frags(skb, frag_iter) { - int end; + return skb_checksum_frag_list(skb, start, offset, len, pos, csum, + csump); +out: + *csump = csum; + return 0; +} +EXPORT_SYMBOL(skb_checksum_full); - WARN_ON(start > offset + len); +/* skb_checksum() has no way to report failure, so it returns 0, which is a + * valid checksum. Use skb_checksum_full() in new code. + */ +__wsum skb_checksum(const struct sk_buff *skb, int offset, int len, __wsum csum) +{ + int err = skb_checksum_full(skb, offset, len, &csum); - end = start + frag_iter->len; - if ((copy = end - offset) > 0) { - __wsum csum2; - if (copy > len) - copy = len; - csum2 = skb_checksum(frag_iter, offset - start, copy, - 0); - csum = csum_block_add(csum, csum2, pos); - if ((len -= copy) == 0) - return csum; - offset += copy; - pos += 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 csum; } EXPORT_SYMBOL(skb_checksum); -- 2.55.0