[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

(racoon 863) Forward: Re: [Ipsec-tools-devel] get_proppair/get_transform bug



KAME racoon also needs to be fixed.

# I think 'if (sa_ret)' is always true though.

-- 
KAMADA Ken'ichi <kamada@nanohz.org>
--- Begin Message ---
On Tue, Nov 16, 2004 at 03:35:42PM +0000, Emmanuel Dreyfus wrote:
> On Tue, Nov 16, 2004 at 08:01:32PM +0530, Ganesan R wrote:
> > +               p->prop = (struct isakmp_pl_p *) racoon_malloc(sizeof(*prop));
> > +               p->trns = (struct isakmp_pl_t *) racoon_malloc(sizeof(*trns));
> 
> As I understand, both payloards are followed by aribtrary length data.

You're right. However, I think we're barking up the wrong tree.
get_proppair() and get_transform() code appears to be perfectly fine. pbuf
is only a parse buffer, the pointer inside each isakmp_parse_t is pointing
to the sa being passed as the first parameter to get_proppair(). This sa
shouldn't get freed before free_proppair() is called. 

I don't understand why efence complains. Here is what I suspect; in
ipsecdoi_checkph2proposal, we have

	rpair = get_proppair(iph2->sa_ret, IPSECDOI_TYPE_PH2);
	
Later, we have code that does	
			     
        /* make a SA to be replayed. */
      	VPTRINIT(iph2->sa_ret);
        iph2->sa_ret = get_sabyproppair(p, iph2->ph1);
	free_proppair0(p);
	if (iph2->sa_ret == NULL)
	      	 goto end;

where, p is a proposal selected from rpair. I think VPTRINIT() is the real
problem. p->prop is still pointing somewhere inside iph2->sa_ret, but we
have freed it already. The patch below should take care of the problem. 

=====
--- ipsec_doi.c.org	2004-11-16 22:59:18.000000000 +0530
+++ ipsec_doi.c	2004-11-16 22:59:58.000000000 +0530
@@ -761,6 +761,7 @@
 	struct prop_pair *p;
 	int i, n, num;
 	int error = -1;
+	vchar_t *sa_ret = NULL;
 
 	/* get proposal pair of SA sent. */
 	spair = get_proppair(iph2->sa, IPSECDOI_TYPE_PH2);
@@ -828,7 +829,7 @@
 		goto end;
 
 	/* make a SA to be replayed. */
-	VPTRINIT(iph2->sa_ret);
+	sa_ret = iph2->sa_ret;
 	iph2->sa_ret = get_sabyproppair(p, iph2->ph1);
 	free_proppair0(p);
 	if (iph2->sa_ret == NULL)
@@ -841,6 +842,8 @@
 		free_proppair(rpair);
 	if (spair)
 		free_proppair(spair);
+	if (sa_ret)
+		vfree(sa_ret);
 
 	return error;
 }

=====

To validate my theory, can you confirm whether efence complains only in
ipsecdoi_checkph2proposal() code path?

Ganesan



-------------------------------------------------------
This SF.Net email is sponsored by: InterSystems CACHE
FREE OODBMS DOWNLOAD - A multidimensional database that combines
robust object and relational technologies, making it a perfect match
for Java, C++,COM, XML, ODBC and JDBC. www.intersystems.com/match8
_______________________________________________
Ipsec-tools-devel mailing list
Ipsec-tools-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipsec-tools-devel

--- End Message ---