Skip to content

Commit e6d8b64

Browse files
Daniel Borkmanndavem330
authored andcommitted
net: sctp: fix and consolidate SCTP checksumming code
This fixes an outstanding bug found through IPVS, where SCTP packets with skb->data_len > 0 (non-linearized) and empty frag_list, but data accumulated in frags[] member, are forwarded with incorrect checksum letting SCTP initial handshake fail on some systems. Linearizing each SCTP skb in IPVS to prevent that would not be a good solution as this leads to an additional and unnecessary performance penalty on the load-balancer itself for no good reason (as we actually only want to update the checksum, and can do that in a different/better way presented here). The actual problem is elsewhere, namely, that SCTP's checksumming in sctp_compute_cksum() does not take frags[] into account like skb_checksum() does. So while we are fixing this up, we better reuse the existing code that we have anyway in __skb_checksum() and use it for walking through the data doing checksumming. This will not only fix this issue, but also consolidates some SCTP code with core sk_buff code, bringing it closer together and removing respectively avoiding reimplementation of skb_checksum() for no good reason. As crc32c() can use hardware implementation within the crypto layer, we leave that intact (it wraps around / falls back to e.g. slice-by-8 algorithm in __crc32c_le() otherwise); plus use the __crc32c_le_combine() combinator for crc32c blocks. Also, we remove all other SCTP checksumming code, so that we only have to use sctp_compute_cksum() from now on; for doing that, we need to transform SCTP checkumming in output path slightly, and can leave the rest intact. Signed-off-by: Daniel Borkmann <dborkman@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 2817a33 commit e6d8b64

File tree

2 files changed

+20
-45
lines changed

2 files changed

+20
-45
lines changed

include/net/sctp/checksum.h

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -42,56 +42,38 @@
4242
#include <linux/types.h>
4343
#include <net/sctp/sctp.h>
4444
#include <linux/crc32c.h>
45+
#include <linux/crc32.h>
4546

46-
static inline __u32 sctp_crc32c(__u32 crc, u8 *buffer, u16 length)
47+
static inline __wsum sctp_csum_update(const void *buff, int len, __wsum sum)
4748
{
48-
return crc32c(crc, buffer, length);
49-
}
50-
51-
static inline __u32 sctp_start_cksum(__u8 *buffer, __u16 length)
52-
{
53-
__u32 crc = ~(__u32)0;
54-
__u8 zero[sizeof(__u32)] = {0};
55-
56-
/* Optimize this routine to be SCTP specific, knowing how
57-
* to skip the checksum field of the SCTP header.
49+
/* This uses the crypto implementation of crc32c, which is either
50+
* implemented w/ hardware support or resolves to __crc32c_le().
5851
*/
59-
60-
/* Calculate CRC up to the checksum. */
61-
crc = sctp_crc32c(crc, buffer, sizeof(struct sctphdr) - sizeof(__u32));
62-
63-
/* Skip checksum field of the header. */
64-
crc = sctp_crc32c(crc, zero, sizeof(__u32));
65-
66-
/* Calculate the rest of the CRC. */
67-
crc = sctp_crc32c(crc, &buffer[sizeof(struct sctphdr)],
68-
length - sizeof(struct sctphdr));
69-
return crc;
70-
}
71-
72-
static inline __u32 sctp_update_cksum(__u8 *buffer, __u16 length, __u32 crc32)
73-
{
74-
return sctp_crc32c(crc32, buffer, length);
52+
return crc32c(sum, buff, len);
7553
}
7654

77-
static inline __le32 sctp_end_cksum(__u32 crc32)
55+
static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
56+
int offset, int len)
7857
{
79-
return cpu_to_le32(~crc32);
58+
return __crc32c_le_combine(csum, csum2, len);
8059
}
8160

82-
/* Calculate the CRC32C checksum of an SCTP packet. */
8361
static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
8462
unsigned int offset)
8563
{
86-
const struct sk_buff *iter;
64+
struct sctphdr *sh = sctp_hdr(skb);
65+
__le32 ret, old = sh->checksum;
66+
const struct skb_checksum_ops ops = {
67+
.update = sctp_csum_update,
68+
.combine = sctp_csum_combine,
69+
};
8770

88-
__u32 crc32 = sctp_start_cksum(skb->data + offset,
89-
skb_headlen(skb) - offset);
90-
skb_walk_frags(skb, iter)
91-
crc32 = sctp_update_cksum((__u8 *) iter->data,
92-
skb_headlen(iter), crc32);
71+
sh->checksum = 0;
72+
ret = cpu_to_le32(~__skb_checksum(skb, offset, skb->len - offset,
73+
~(__u32)0, &ops));
74+
sh->checksum = old;
9375

94-
return sctp_end_cksum(crc32);
76+
return ret;
9577
}
9678

9779
#endif /* __sctp_checksum_h__ */

net/sctp/output.c

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,6 @@ int sctp_packet_transmit(struct sctp_packet *packet)
390390
__u8 has_data = 0;
391391
struct dst_entry *dst = tp->dst;
392392
unsigned char *auth = NULL; /* pointer to auth in skb data */
393-
__u32 cksum_buf_len = sizeof(struct sctphdr);
394393

395394
pr_debug("%s: packet:%p\n", __func__, packet);
396395

@@ -493,7 +492,6 @@ int sctp_packet_transmit(struct sctp_packet *packet)
493492
if (chunk == packet->auth)
494493
auth = skb_tail_pointer(nskb);
495494

496-
cksum_buf_len += chunk->skb->len;
497495
memcpy(skb_put(nskb, chunk->skb->len),
498496
chunk->skb->data, chunk->skb->len);
499497

@@ -538,12 +536,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)
538536
if (!sctp_checksum_disable) {
539537
if (!(dst->dev->features & NETIF_F_SCTP_CSUM) ||
540538
(dst_xfrm(dst) != NULL) || packet->ipfragok) {
541-
__u32 crc32 = sctp_start_cksum((__u8 *)sh, cksum_buf_len);
542-
543-
/* 3) Put the resultant value into the checksum field in the
544-
* common header, and leave the rest of the bits unchanged.
545-
*/
546-
sh->checksum = sctp_end_cksum(crc32);
539+
sh->checksum = sctp_compute_cksum(nskb, 0);
547540
} else {
548541
/* no need to seed pseudo checksum for SCTP */
549542
nskb->ip_summed = CHECKSUM_PARTIAL;

0 commit comments

Comments
 (0)