On Tue, Nov 03, 1998 at 04:51:34PM +0000, David Woodhouse wrote:
> Thanks to Juanjo and Andi, I think this problem is now fixed.
>
> I think both of you simultaneously came up with the idea that the offset of
> the data payload (proto_doff) could be negative, causing csum_partial() to
> checksum the whole of physical memory before oopsing.
>
This patch fixes incorrect ip_masq csuming when faced with corrupted pkts.
It have been succesfully tested by David "tortured-by-Oops" Woodhouse,
his Oops have stopped so apparently it did fix the problem
(_apart_ from the "apparent-fix" its a REAL code fix).
Linus, could you please apply it?
-- -- Juanjo http://juanjox.home.ml.org/== free collective power ==---. Linux <------------'
--BOKacYhQ+x31HxR3 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="ip_masq-doff-v3.patch"
Basically, this patch does an ``anal-tappo'' protocol data size check at ip_fw_*masq() entry path.
--- linux/net/ipv4/ip_masq.c.dist Fri Oct 23 10:39:57 1998 +++ linux/net/ipv4/ip_masq.c Tue Nov 3 17:15:36 1998 @@ -337,6 +337,9 @@ #define PORT_MASQ_MUL 10 #endif +/* + * At the moment, hardcore in sync with masq_proto_num + */ atomic_t ip_masq_free_ports[3] = { ATOMIC_INIT((PORT_MASQ_END-PORT_MASQ_BEGIN) * PORT_MASQ_MUL),/* UDP */ ATOMIC_INIT((PORT_MASQ_END-PORT_MASQ_BEGIN) * PORT_MASQ_MUL),/* TCP */ @@ -960,15 +963,40 @@ return NULL; } -static __inline__ unsigned proto_doff(unsigned proto, char *th) +/* + * Get transport protocol data offset, check against size + */ +static __inline__ int proto_doff(unsigned proto, char *th, unsigned size) { + unsigned ret = -1; switch (proto) { + case IPPROTO_ICMP: + if (size >= sizeof(struct icmphdr)) + ret = sizeof(struct icmphdr); + break; case IPPROTO_UDP: - return sizeof(struct udphdr); + if (size >= sizeof(struct udphdr)) + ret = sizeof(struct udphdr); + break; case IPPROTO_TCP: - return ((struct tcphdr*)th)->doff << 2; + /* + * Is this case, this check _also_ avoids + * touching an invalid pointer if + * size is invalid + */ + if (size >= sizeof(struct tcphdr)) { + ret = ((struct tcphdr*)th)->doff << 2; + if (ret > size) { + ret = -1 ; + } + } + + break; } - return 0; + if (ret < 0) + IP_MASQ_DEBUG(0, "mess proto_doff for proto=%d, size =%d\n", + proto, size); + return ret; } int ip_fw_masquerade(struct sk_buff **skb_p, __u32 maddr) @@ -980,19 +1008,26 @@ int size; /* - * Magic "doff" csum semantics - * !0: saved payload csum IS valid, doff is correct - * 0: csum not valid + * doff holds transport protocol data offset + * csum holds its checksum + * csum_ok says if csum is valid */ - unsigned doff = 0; + int doff = 0; int csum = 0; + int csum_ok = 0; /* * We can only masquerade protocols with ports... and hack some ICMPs */ h.raw = (char*) iph + iph->ihl * 4; + size = ntohs(iph->tot_len) - (iph->ihl * 4); + doff = proto_doff(iph->protocol, h.raw, size); + if (doff < 0) { + IP_MASQ_DEBUG(0, "O-pkt invalid packet data size\n"); + return -1; + } switch (iph->protocol) { case IPPROTO_ICMP: return(ip_fw_masq_icmp(skb_p, maddr)); @@ -1002,7 +1037,6 @@ break; case IPPROTO_TCP: /* Make sure packet is in the masq range */ - size = ntohs(iph->tot_len) - (iph->ihl * 4); IP_MASQ_DEBUG(3, "O-pkt: %s size=%d\n", masq_proto_name(iph->protocol), size); @@ -1011,7 +1045,7 @@ switch (skb->ip_summed) { case CHECKSUM_NONE: - doff = proto_doff(iph->protocol, h.raw); + csum_ok++; csum = csum_partial(h.raw + doff, size - doff, 0); IP_MASQ_DEBUG(3, "O-pkt: %s I-datacsum=%d\n", masq_proto_name(iph->protocol), @@ -1133,7 +1167,7 @@ */ if (ms->app) - doff = 0; + csum_ok = 0; /* * Attempt ip_masq_app call. @@ -1148,6 +1182,7 @@ iph = skb->nh.iph; h.raw = (char*) iph + iph->ihl *4; size = skb->len - (h.raw - skb->nh.raw); + /* doff should have not changed */ } /* @@ -1158,8 +1193,7 @@ * Transport's payload partial csum */ - if (!doff) { - doff = proto_doff(iph->protocol, h.raw); + if (!csum_ok) { csum = csum_partial(h.raw + doff, size - doff, 0); } skb->csum = csum; @@ -1710,8 +1744,9 @@ union ip_masq_tphdr h; struct ip_masq *ms; unsigned short size; - unsigned doff = 0; + int doff = 0; int csum = 0; + int csum_ok = 0; __u32 maddr; /* @@ -1727,8 +1762,19 @@ return 0; } - maddr = iph->daddr; h.raw = (char*) iph + iph->ihl * 4; + /* + * IP payload size + */ + size = ntohs(iph->tot_len) - (iph->ihl * 4); + + doff = proto_doff(iph->protocol, h.raw, size); + if (doff < 0) { + IP_MASQ_DEBUG(0, "I-pkt invalid packet data size\n"); + return -1; + } + + maddr = iph->daddr; switch (iph->protocol) { case IPPROTO_ICMP: @@ -1749,7 +1795,6 @@ return 0; /* Check that the checksum is OK */ - size = ntohs(iph->tot_len) - (iph->ihl * 4); if ((iph->protocol == IPPROTO_UDP) && (h.uh->check == 0)) /* No UDP checksum */ break; @@ -1757,8 +1802,8 @@ switch (skb->ip_summed) { case CHECKSUM_NONE: - doff = proto_doff(iph->protocol, h.raw); csum = csum_partial(h.raw + doff, size - doff, 0); + csum_ok++; skb->csum = csum_partial(h.raw , doff, csum); case CHECKSUM_HW: @@ -1846,7 +1891,7 @@ */ if (ms->app) - doff = 0; + csum_ok = 0; /* * Attempt ip_masq_app call. @@ -1873,8 +1918,7 @@ * Transport's payload partial csum */ - if (!doff) { - doff = proto_doff(iph->protocol, h.raw); + if (!csum_ok) { csum = csum_partial(h.raw + doff, size - doff, 0); } skb->csum = csum;
--BOKacYhQ+x31HxR3--
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/