lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <mGT4tZUhbVI4gq_S5IVRL2jaA5f241EsnImGDuVoNpK2rXixwiekd6GeYYGLUdyUm2ZA22CNb8j5wlBbUMjATfsOmEMma2K3REA7nfEgFzk=@protonmail.com> Date: Sun, 29 Dec 2019 02:13:09 +0000 From: Ttttabcd <ttttabcd@...tonmail.com> To: David Miller <davem@...emloft.net> Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "kuznet@....inr.ac.ru" <kuznet@....inr.ac.ru>, "yoshfuji@...ux-ipv6.org" <yoshfuji@...ux-ipv6.org> Subject: Re: [PATCH] Improved handling of incorrect IP fragments In both ip6_frag_queue and ip6_frag_reasm, it is checked whether it is an Oversized IPv6 packet, which is duplicated. The original code logic will only be processed in ip6_frag_queue. The code of ip6_frag_reasm will not be executed. Now change it to only process in ip6_frag_queue and output the prompt information. In ip6_frag_queue, set the prob_offset pointer to notify ipv6_frag_rcv to send the ICMPv6 parameter problem message. The logic is not concise, I merged the part that sent ICMPv6 messages into ip6_frag_queue. In the original logic, receiving oversized IPv6 fragments and receiving IPv6 fragments whose end is not an integral multiple of 8 bytes both returns -1. This is inconsistent with other incorrect IPv6 fragmentation processing. For example, in other logic, end == offset will goto discard_fq directly. I think that receiving any incorrect IPv6 fragment means that the fragment processing has failed as a whole, and the data carried in the fragments is likely to be wrong. I think we should also execute goto discard_fq to release this fragments queue. Goto discard_fq at the end of the label send_param_prob, release the fragment queue. Since icmpv6_param_prob will call kfree_skb, it will also be kfree_skb in the label err, which will cause repeated free, so I added skb_get. I also made similar changes in IPv4 fragmentation processing. It is not good to use 65535 values directly, I added the IPV4_MAX_TOT_LEN macro. The oversized check in IPv4 fragment processing is in the ip_frag_reasm of the reassembly fragment. This is too late. The incorrect IP fragment has been inserted into the fragment queue. I modified it in ip_frag_queue. I changed the original net_info_ratelimited to net_dbg_ratelimited to make the debugging information more controllable. I also modified goto discard_qp directly when the end is not an integer multiple of 8 bytes. Signed-off-by: AK Deng <ttttabcd@...tonmail.com> --- include/net/ip.h | 2 ++ net/ipv4/ip_fragment.c | 20 +++++++++---------- net/ipv6/reassembly.c | 45 ++++++++++++++++++++---------------------- 3 files changed, 33 insertions(+), 34 deletions(-) diff --git a/include/net/ip.h b/include/net/ip.h index 5b317c9f4470..43e9dc51852b 100644 --- a/include/net/ip.h +++ b/include/net/ip.h @@ -34,6 +34,8 @@ #define IPV4_MAX_PMTU 65535U /* RFC 2675, Section 5.1 */ #define IPV4_MIN_MTU 68 /* RFC 791 */ +#define IPV4_MAX_TOT_LEN 65535 + extern unsigned int sysctl_fib_sync_mem; extern unsigned int sysctl_fib_sync_mem_min; extern unsigned int sysctl_fib_sync_mem_max; diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c index cfeb8890f94e..baee3383d0ac 100644 --- a/net/ipv4/ip_fragment.c +++ b/net/ipv4/ip_fragment.c @@ -300,6 +300,12 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) end = offset + skb->len - skb_network_offset(skb) - ihl; err = -EINVAL; + if ((unsigned int)end + ihl > IPV4_MAX_TOT_LEN) { + net_dbg_ratelimited("ip_frag_queue: Oversized IP packet from %pI4, end = %d\n", + &qp->q.key.v4.saddr, end); + goto discard_qp; + } + /* Is this the final fragment? */ if ((flags & IP_MF) == 0) { /* If we already have some bits beyond end @@ -311,11 +317,10 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) qp->q.flags |= INET_FRAG_LAST_IN; qp->q.len = end; } else { - if (end&7) { - end &= ~7; - if (skb->ip_summed != CHECKSUM_UNNECESSARY) - skb->ip_summed = CHECKSUM_NONE; - } + /* Check if the fragment is rounded to 8 bytes. */ + if (end & 0x7) + goto discard_qp; + if (end > qp->q.len) { /* Some bits beyond end -> corruption. */ if (qp->q.flags & INET_FRAG_LAST_IN) @@ -423,8 +428,6 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb, len = ip_hdrlen(skb) + qp->q.len; err = -E2BIG; - if (len > 65535) - goto out_oversize; inet_frag_reasm_finish(&qp->q, skb, reasm_data, ip_frag_coalesce_ok(qp)); @@ -462,9 +465,6 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb, out_nomem: net_dbg_ratelimited("queue_glue: no memory for gluing queue %p\n", qp); err = -ENOMEM; - goto out_fail; -out_oversize: - net_info_ratelimited("Oversized IP packet from %pI4\n", &qp->q.key.v4.saddr); out_fail: __IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS); return err; diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c index 1f5d4d196dcc..302bf6c26c45 100644 --- a/net/ipv6/reassembly.c +++ b/net/ipv6/reassembly.c @@ -102,13 +102,13 @@ fq_find(struct net *net, __be32 id, const struct ipv6hdr *hdr, int iif) } static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb, - struct frag_hdr *fhdr, int nhoff, - u32 *prob_offset) + struct frag_hdr *fhdr, int nhoff) { struct net *net = dev_net(skb_dst(skb)->dev); int offset, end, fragsize; struct sk_buff *prev_tail; struct net_device *dev; + u32 prob_offset = 0; int err = -ENOENT; u8 ecn; @@ -121,11 +121,10 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb, ((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1))); if ((unsigned int)end > IPV6_MAXPLEN) { - *prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb); - /* note that if prob_offset is set, the skb is freed elsewhere, - * we do not free it here. - */ - return -1; + prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb); + net_dbg_ratelimited("ip6_frag_queue: Oversized IPv6 packet from %pI6c, end = %d\n", + &fq->q.key.v6.saddr, end); + goto send_param_prob; } ecn = ip6_frag_ecn(ipv6_hdr(skb)); @@ -155,8 +154,8 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb, /* RFC2460 says always send parameter problem in * this case. -DaveM */ - *prob_offset = offsetof(struct ipv6hdr, payload_len); - return -1; + prob_offset = offsetof(struct ipv6hdr, payload_len); + goto send_param_prob; } if (end > fq->q.len) { /* Some bits beyond end -> corruption. */ @@ -236,6 +235,18 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb, err: kfree_skb(skb); return err; +send_param_prob: + __IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev), + IPSTATS_MIB_INHDRERRORS); + /* icmpv6_param_prob() calls kfree_skb(skb), + * and the label err also calls kfree_skb(skb), + * so skb_get(skb) here increases the reference count + * to avoid duplicate release + */ + skb_get(skb); + icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset); + err = -1; + goto discard_fq; } /* @@ -267,8 +278,6 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, payload_len = ((skb->data - skb_network_header(skb)) - sizeof(struct ipv6hdr) + fq->q.len - sizeof(struct frag_hdr)); - if (payload_len > IPV6_MAXPLEN) - goto out_oversize; /* We have to remove fragment header from datagram and to relocate * header in order to calculate ICV correctly. */ @@ -303,9 +312,6 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, fq->q.last_run_head = NULL; return 1; -out_oversize: - net_dbg_ratelimited("ip6_frag_reasm: payload len = %d\n", payload_len); - goto out_fail; out_oom: net_dbg_ratelimited("ip6_frag_reasm: no memory for reassembly\n"); out_fail: @@ -354,23 +360,14 @@ static int ipv6_frag_rcv(struct sk_buff *skb) iif = skb->dev ? skb->dev->ifindex : 0; fq = fq_find(net, fhdr->identification, hdr, iif); if (fq) { - u32 prob_offset = 0; int ret; - spin_lock(&fq->q.lock); fq->iif = iif; - ret = ip6_frag_queue(fq, skb, fhdr, IP6CB(skb)->nhoff, - &prob_offset); + ret = ip6_frag_queue(fq, skb, fhdr, IP6CB(skb)->nhoff); spin_unlock(&fq->q.lock); inet_frag_put(&fq->q); - if (prob_offset) { - __IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev), - IPSTATS_MIB_INHDRERRORS); - /* icmpv6_param_prob() calls kfree_skb(skb) */ - icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset); - } return ret; } -- 2.24.0
Powered by blists - more mailing lists