[vchkpw] [SPAM] Bug in maildirquota.c

2014-01-31 Thread Simone Lazzaris
Hi everybody.

I've been hit by a bug in vdelivermail (in maildirquota.c, precisely), and I 
want to share the
resolution I've found.


I've recently upgraded to vpopmail 5.4.33, and we experienced that occasionally 
(once o twice a day) vdelivermail starts looping, eating all the CPU.

An strace on the offending instance resulted in:
read(5, 0xf5e4317, 2963227893)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4316, 2963227894)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4315, 2963227895)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4314, 2963227896)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4313, 2963227897)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4312, 2963227898)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4311, 2963227899)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4310, 2963227900)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e430f, 2963227901)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e430e, 2963227902)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e430d, 2963227903)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e430c, 2963227904)  = -1 EINVAL (Invalid argument)
...
...

The file descriptor number 5 of that process is pointing at the maildirsize 
file of the mailbox, and is marked as deleted in /proc/procid/fd

We store the mail in a NetApp NFS share, without locking for performance 
reasons, and I think I've found the problem:
the file is deleted by someone else while vdelivermail is reading it.

In maildirquota.c, function maildirsize_read, there is a while loop that reads:
while (l)
{
n=read(f, p, l);
if (n  0)
{

But n is defined as a unsigned int (64 bit) , so even if read returns a 
negative value (error) the if is never trigged.

So I've made this patch:
--- maildirquota.c.orig 2014-01-31 12:21:22.0 +0100
+++ maildirquota.c  2014-01-31 12:08:47.0 +0100
@@ -337,7 +337,6 @@
  int f;
  char *p;
  unsigned l;
- storage_t n;
  int first;
  int ret = 0;
 
@@ -360,15 +359,16 @@
 
while (l)
{
-   n=read(f, p, l);
-   if (n  0)
+   ssize_t nr;
+   nr=read(f, p, l);
+   if (nr  0)
{
close(f);
return (-1);
}
-   if (n == 0) break;
-   p += n;
-   l -= n;
+   if (nr == 0)break;
+   p += nr;
+   l -= nr;
}
if (l == 0 || ret)  /* maildir too big */
{

which fixes the problem.


Any chance to incorporate the fix in the next version ?


Thanks


-- 
Simone Lazzaris
QCom S.p.A.

!DSPAM:52eb8a1334261024417156!



[vchkpw] [SPAM] Bug in maildirquota.c

2014-01-31 Thread Simone Lazzaris
Hi everybody.

I've been hit by a bug in vdelivermail (in maildirquota.c, precisely), and I 
want to share the
resolution I've found.


I've recently upgraded to vpopmail 5.4.33, and we experienced that occasionally 
(once o twice a day) vdelivermail starts looping, eating all the CPU.

An strace on the offending instance resulted in:
read(5, 0xf5e4317, 2963227893)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4316, 2963227894)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4315, 2963227895)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4314, 2963227896)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4313, 2963227897)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4312, 2963227898)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4311, 2963227899)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e4310, 2963227900)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e430f, 2963227901)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e430e, 2963227902)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e430d, 2963227903)  = -1 EINVAL (Invalid argument)
read(5, 0xf5e430c, 2963227904)  = -1 EINVAL (Invalid argument)
...
...

The file descriptor number 5 of that process is pointing at the maildirsize 
file of the mailbox, and is marked as deleted in /proc/procid/fd

We store the mail in a NetApp NFS share, without locking for performance 
reasons, and I think I've found the problem:
the file is deleted by someone else while vdelivermail is reading it.

In maildirquota.c, function maildirsize_read, there is a while loop that reads:
while (l)
{
n=read(f, p, l);
if (n  0)
{

But n is defined as a unsigned int (64 bit) , so even if read returns a 
negative value (error) the if is never trigged.

So I've made this patch:
--- maildirquota.c.orig 2014-01-31 12:21:22.0 +0100
+++ maildirquota.c  2014-01-31 12:08:47.0 +0100
@@ -337,7 +337,6 @@
  int f;
  char *p;
  unsigned l;
- storage_t n;
  int first;
  int ret = 0;
 
@@ -360,15 +359,16 @@
 
while (l)
{
-   n=read(f, p, l);
-   if (n  0)
+   ssize_t nr;
+   nr=read(f, p, l);
+   if (nr  0)
{
close(f);
return (-1);
}
-   if (n == 0) break;
-   p += n;
-   l -= n;
+   if (nr == 0)break;
+   p += nr;
+   l -= nr;
}
if (l == 0 || ret)  /* maildir too big */
{

which fixes the problem.


Any chance to incorporate the fix in the next version ?


Thanks


-- 
Simone Lazzaris
QCom S.p.A.

!DSPAM:52eb8b0234261033718879!