Hi,

since some popular client (we know which one ;) sometimes just kicks a 
connection instead of gracefully closing it we have a decent number of broken 
pipe signals sent to our imapds.

since our upgrade from 2.0 to 2.1.11 this was often followed by a segfault of 
the process who just got the broken pipe.

since I don't like to run programs which segfault I did a little debugging:

If the broken pipe is encountered during write (prot_flush) the process 
doesn't immediately close the connection but finishes the current command; 
s->cnt stays 0 and s->ptr isn't set to the buffer start.

If prot_putc is called later on (during finishing the current command output), 
it decreases s->cnt to -1 and writes after the malloced buffer. The #defined 
version of prot_putc isn't guarded by an assertion to defend this (it took me 
a decent debugging session to find out about the #defined version...).

If there are enough calls to prot_putc you'll get a nice segfault.

The attached patch fixes this.

While I was at it I took a look at the other prot_ functions. I think at least 
prot_ungetc should be guarded against buffer overflows too.

Kind regards,

Gerd
diff -r -u cyrus-imapd-2.1.11.orig/lib/prot.c cyrus-imapd-2.1.11/lib/prot.c
--- cyrus-imapd-2.1.11.orig/lib/prot.c	Mon Oct 21 22:44:22 2002
+++ cyrus-imapd-2.1.11/lib/prot.c	Sat Jan  4 23:19:56 2003
@@ -793,6 +793,7 @@
 int prot_putc(int c, struct protstream *s)
 {
     assert(s->write);
+    if(s->error || s->eof) return EOF;
     assert(s->cnt > 0);
 
     *s->ptr++ = c;
diff -r -u cyrus-imapd-2.1.11.orig/lib/prot.h cyrus-imapd-2.1.11/lib/prot.h
--- cyrus-imapd-2.1.11.orig/lib/prot.h	Tue Apr  2 05:59:04 2002
+++ cyrus-imapd-2.1.11/lib/prot.h	Sat Jan  4 23:21:55 2003
@@ -104,7 +104,7 @@
 
 #define prot_getc(s) ((s)->cnt-- > 0 ? (int)*(s)->ptr++ : prot_fill(s))
 #define prot_ungetc(c, s) ((s)->cnt++, (*--(s)->ptr = (c)))
-#define prot_putc(c, s) ((*(s)->ptr++ = (c)), --(s)->cnt == 0 ? prot_flush(s) : 0)
+#define prot_putc(c, s) (!((s)->error || (s)->eof) ? ((*(s)->ptr++ = (c)), --(s)->cnt == 0 ? prot_flush(s) : 0) : 0)
 #define prot_BLOCK(s) ((s)->dontblock = 0)
 #define prot_NONBLOCK(s) ((s)->dontblock = 1)
 

Reply via email to