Re: [BK PATCH] Fix ip_conntrack_amanda data corruption bug that breaksamanda dumps

[Pablo Neira]

Patrick McHardy wrote:

> Matthias Andree wrote:
>
> > On Thu, 04 Nov 2004, Patrick McHardy wrote:
> >
> > > The data that is changed is only a copy, the actual packet is not 
> > > touched.
> > >   
> >
> >
> > Why then does the application not see the packets as long as
> > ip_conntrack_amanda is loaded and starts seeing them again as soon as
> > "rmmod ip_conntrack_amanda" has completed?
> >  
>
> Your observation and your patch were correct, thanks. It is supposed
> to be just a copy, I missed that it wasn't anymore. While your patch
> works too, and is even faster with non-linear skbs, I don't like the
> idea of using the skb as a scratch-area, so I sent this patch to Dave
> instead.


Patrick, what about this? this way we save a copy to a buffer for linear 
skbs.

Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxx>
===== net/ipv4/netfilter/ip_conntrack_amanda.c 1.10 vs edited =====
--- 1.10/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-08-19 02:14:53 +02:00
+++ edited/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-11-05 17:32:04 +01:00
@@ -49,9 +49,10 @@
 {
 	struct ip_conntrack_expect *exp;
 	struct ip_ct_amanda_expect *exp_amanda_info;
-	char *amp, *data, *data_limit, *tmp;
+	char *amp, *data, *tmp;
 	unsigned int dataoff, i;
 	u_int16_t port, len;
+	int found = 0;
 
 	/* Only look at packets from the Amanda server */
 	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
@@ -74,12 +75,17 @@
 				 skb->len - dataoff, amanda_buffer);
 	BUG_ON(amp == NULL);
 	data = amp;
-	data_limit = amp + skb->len - dataoff;
-	*data_limit = '\0';
 
 	/* Search for the CONNECT string */
-	data = strstr(data, "CONNECT ");
-	if (!data)
+	while((data = memchr(data, 'C', skb->len - dataoff)) != NULL) {
+		if (strncmp(data, "CONNECT ", 8) == 0) {
+			found = 1;
+			break;
+		}
+		data++;
+	}
+
+	if (!found)
 		goto out;
 	data += strlen("CONNECT ");