In message <200107312226.CAA00407@xxxxxxxxxxxxxx> you write:
> Very interesting... Paul, did he really not miss the place where it was
> really checked?
The bug is elsewhere. Looks like Marc forgot that we don't do NAT if
the conntrack code hasn't already set skb->nfct. 8)
If the packet gets here, it is an ICMP packet which the connection
tracking code has marked RELATED.
The only way ICMP packets get marked RELATED is in icmp_error_track.
To do this, it has to get past ip_conntrack_core.c:357-359:
datalen = skb->len - iph->ihl*4 - sizeof(*hdr);
if (skb->len < iph->ihl * 4 + sizeof(struct icmphdr)) {
DEBUGP("icmp_error_track: too short\n");
return NULL;
}
In my first audit, I noticed that this should strictly be ... +
sizeof(struct iphdr), but we only read from it.
Anyway, so datalen is always >= 0.
And this is the killer: line 385 (it's redundant: we check this inside
get_tuple anyway):
/* Are they talking about one of our connections? */
if (inner->ihl * 4 + 8 > datalen
|| !get_tuple(inner, datalen, &origtuple, innerproto)) {
So, we will always have 8 protocol bytes in the inner packet. This is
enough to contain the source and destinations ports (TCP/UDP) or ICMP
id, so we're not writing over the end of the packet...
Now, if Marc is seeing this, and noone can find a flaw in my logic,
then I'd say that someone else is mangling a packet without doing (see
ipip.c):
nf_conntrack_put(skb->nfct);
skb->nfct = NULL;
Please find them and hit them hard...
Hope that helps,
Rusty.
--
Premature optmztion is rt of all evl. --DK
|