Yahoo Groups archive

Milter-greylist

Index last updated: 2026-04-28 23:32 UTC

Message

Re: [milter-greylist] crash of milter-greylist process due to stack corruption [2 Attachments]

2015-06-07 by manu@...

'Bruncsak, Attila' attila.bruncsak@... [milter-greylist]
<milter-greylist@yahoogroups.com> wrote:

> In the function sync_waitdata() the value of "fd" is not checked
> against of FD_SETSIZE. It must have a value smaller than that.
> 
> Please apply the patch attached to that message.

Is there any merit to keep the select() code along with the poll() flavor? 
I would rather replace it all (like below). What is your opinion?

Index: sync.c
===================================================================
RCS file: /cvsroot/milter-greylist/sync.c,v
retrieving revision 1.90
diff -U 4 -r1.90 sync.c
--- sync.c      23 Mar 2013 01:45:02 -0000      1.90
+++ sync.c      7 Jun 2015 04:30:14 -0000
@@ -47,8 +47,9 @@
 #include <strings.h>
 #include <pthread.h>
 #include <syslog.h>
 #include <sysexits.h>
+#include <poll.h>
 
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
@@ -1316,34 +1317,48 @@
 sync_waitdata(fd, stimeout)
        int fd;
        time_t stimeout;
 {
-       fd_set fdr, fde;
-       struct timeval timeout;
+       struct pollfd fds;
        int retval;
 
-       FD_ZERO(&fdr);
-       FD_SET(fd, &fdr);
+       if (stimeout == 0)
+               stimeout = INFTIM;
+       else
+               stimeout = 1000 * stimeout;
 
-       FD_ZERO(&fde);
-       FD_SET(fd, &fde);
+       fds.fd = fd;
+       fds.events = POLLIN;
+       
+       retval = poll(&fds, 1, stimeout);
 
-       timeout.tv_sec = stimeout;
-       timeout.tv_usec = 0;
+       switch (retval) {
+       case 0:
+               mg_log(LOG_ERR, "%s: poll timeout", __func__);
+               break;
 
-       retval = select(fd + 1, &fdr, NULL, &fde, stimeout == 0 ? NULL : &timeout); 
+       case -1:
+               mg_log(LOG_ERR, "%s: poll error: %s",
+                      __func__, strerror(errno));
+               break;
 
-       if (retval == 0) {
-               mg_log(LOG_ERR, "sync_waitdata : select timeout");
-               
-       } else {
-               if (retval < 0) {
-                       mg_log(LOG_ERR, "sync_waitdata : error");
-               } else {
-                       if (FD_ISSET(fd,&fde)) {
-                               mg_log(LOG_ERR, "sync_waitdata : exception fd is
set");
-                       }
-               }
+       default:
+               if (fds.revents & POLLPRI)
+                       mg_log(LOG_ERR, "%s: high priority data available",
+                              __func__);
+
+               if (fds.revents & POLLERR)
+                       mg_log(LOG_ERR, "%s: exceptional condition",
+                              __func__);
+
+               if (fds.revents & POLLHUP)
+                       mg_log(LOG_ERR, "%s: socket was disconnected",
+                              __func__);
+
+               if (fds.revents & POLLHUP)
+                       mg_log(LOG_ERR, "%s: file descriptor not open",
+                              __func__);
+               break;
        }
 
        return retval;
 }


-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

Attachments

Move to quarantaine

This moves the raw source file on disk only. The archive index is not changed automatically, so you still need to run a manual refresh afterward.