CSTP reconnect segfault on HEAD

[David Woodhouse]

On Wed, 2012-06-27 at 12:09 -0500, Jack Miller wrote:
> #0  queue_packet (q=<optimized out>, q at entry=0x62fd78, new=0x0)
>     at mainloop.c:40
> #1  0x00000000004081fb in cstp_reconnect (vpninfo=vpninfo at entry=0x62f7c0)
>     at cstp.c:535 

Thanks for the clear report. It's rare that a bug report leads me
straight to the fix, and it's nice when it happens.

The problem is in cstp_reconnect() in frame #1, and it arises because
you have --no-deflate on the command line. (Why, btw?)

On reconnect, we check if the pending packet to be sent is the special
'deflate_pkt', which contains the compressed version of a packet. If so,
we requeue the *original* packet, since the compression will reset with
the new connection.

However, the check 'vpninfo->current_ssl_pkt == vpninfo->deflate_pkt' is
still true even when both of those pointers are NULL, because there's no
pending packet to be sent and the 'deflate_pkt' was never allocated
because we aren't doing compression today. So we try to requeue the
packet, which is NULL...

Your patch was actually perfectly sufficient, but this is the better fix
which I'll push to the repository if you can confirm that it's working:

From 7d9bf34daa595da0d876bd594f91e2ee947f398b Mon Sep 17 00:00:00 2001
From: David Woodhouse <David.Woodhouse at intel.com>
Date: Wed, 27 Jun 2012 19:58:55 +0100
Subject: [PATCH] Fix SEGV on cstp_reconnect() without deflate

Signed-off-by: David Woodhouse <David.Woodhouse at intel.com>
---
 cstp.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/cstp.c b/cstp.c
index ab5eba9..4da4698 100644
--- a/cstp.c
+++ b/cstp.c
@@ -529,13 +529,13 @@ int cstp_reconnect(struct openconnect_info *vpninfo)
 
 	openconnect_close_https(vpninfo, 0);
 
-	/* Requeue the original packet that was deflated */
-	if (vpninfo->current_ssl_pkt == vpninfo->deflate_pkt) {
-		vpninfo->current_ssl_pkt = NULL;
-		queue_packet(&vpninfo->outgoing_queue, vpninfo->pending_deflated_pkt);
-		vpninfo->pending_deflated_pkt = NULL;
-	}
 	if (vpninfo->deflate) {
+		/* Requeue the original packet that was deflated */
+		if (vpninfo->current_ssl_pkt == vpninfo->deflate_pkt) {
+			vpninfo->current_ssl_pkt = NULL;
+			queue_packet(&vpninfo->outgoing_queue, vpninfo->pending_deflated_pkt);
+			vpninfo->pending_deflated_pkt = NULL;
+		}
 		inflateEnd(&vpninfo->inflate_strm);
 		deflateEnd(&vpninfo->deflate_strm);
 	}
-- 
1.7.10.2



-- 
dwmw2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6171 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20120627/a4a988ac/attachment.bin>