lkml.org 
[lkml]   [1998]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: Either the IP masq code or my brain is broken.
On Thu, Oct 29, 1998 at 10:45:03PM +0100, Andi Kleen wrote:
> On Tue, Oct 27, 1998 at 12:01:24PM +0100, David Woodhouse wrote:
> > > For what address exactly does the oops occur?
> >
> > I've got three oopsen on record (of my own). Of those, one was 'page fault
> > from irq handler', and the other two were oopsing on the address c2000000.
> [...]
>
> Could you check if this patch solves the problem?
>
:
: [SNIP]
:
> + datasz = size - doff;
> + if (datasz < 0)
> + return -1;
> + csum = csum_partial(h.raw + doff, datasz, 0);
:
:
The check is necesary but not sufficient.
In proto_doff() there is an unchecked th->doff access that needs also
to be performed.

Could ppl having problems try attached patch?

BTW now I wonder how may masq (miserably) handle fragments [*]; given
that in/out association is key-ed by protocol ports, it should give up
on frags (currently it does not takes care).

[*] supposing brain-damaged CONFIG_IP_MASQUERADE=y and CONFIG_ALWAYS_DEFRAG=n


Regards.
--
-- Juanjo http://juanjox.home.ml.org/

== free collective power ==---.
Linux <------------'

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 Wed Oct 28 19:14:41 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_WARNING("O-pkt invalid data packet\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_WARNING("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;
\
 
 \ /
  Last update: 2005-03-22 13:45    [W:0.048 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site