| 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