Hi Davide,
On Thu, May 18, 2017 at 06:01:43PM +0200, Davide Caratti wrote:
> sctp_compute_cksum() implementation assumes that at least the SCTP header
> is in the linear part of skb: modify conntrack error callback to avoid
> false CRC32c mismatch, if the transport header is partially/entirely paged.
I guess you considered this, but I would like to know the reason for
this approach. Why not fix this from sctp_compute_cksum()? I mean, I
can see other spots in the kernel tree that may be affected by this?
Or is it that you're only observing this from a path that is specific
of conntrack?
> Signed-off-by: Davide Caratti <dcaratti@xxxxxxxxxx>
> ---
> net/netfilter/nf_conntrack_proto_sctp.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
> index 13875d5..1c5b14a 100644
> --- a/net/netfilter/nf_conntrack_proto_sctp.c
> +++ b/net/netfilter/nf_conntrack_proto_sctp.c
> @@ -512,16 +512,19 @@ static int sctp_error(struct net *net, struct nf_conn *tpl, struct sk_buff *skb,
> u8 pf, unsigned int hooknum)
> {
> const struct sctphdr *sh;
> - struct sctphdr _sctph;
> const char *logmsg;
>
> - sh = skb_header_pointer(skb, dataoff, sizeof(_sctph), &_sctph);
> - if (!sh) {
> + if (skb->len < dataoff + sizeof(struct sctphdr)) {
> logmsg = "nf_ct_sctp: short packet ";
> goto out_invalid;
> }
> if (net->ct.sysctl_checksum && hooknum == NF_INET_PRE_ROUTING &&
> skb->ip_summed == CHECKSUM_NONE) {
> + if (!skb_make_writable(skb, dataoff + sizeof(struct sctphdr))) {
> + logmsg = "nf_ct_sctp: failed to read header ";
> + goto out_invalid;
> + }
> + sh = (const struct sctphdr *)(skb->data + dataoff);
> if (sh->checksum != sctp_compute_cksum(skb, dataoff)) {
> logmsg = "nf_ct_sctp: bad CRC ";
> goto out_invalid;
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
