summaryrefslogtreecommitdiffabout
authorSergey Poznyakoff <gray@gnu.org.ua>2007-04-16 20:07:43 (GMT)
committer Sergey Poznyakoff <gray@gnu.org.ua>2007-04-16 20:07:43 (GMT)
commit2cde838634410277e59562959ed3ee9989622d0d (patch) (side-by-side diff)
tree17ec9feb185375944c49405644258739984f7269
parent7fa5d3c0edad565d629648fe7ffa490cd2f559d4 (diff)
downloadmailfromd-2cde838634410277e59562959ed3ee9989622d0d.tar.gz
mailfromd-2cde838634410277e59562959ed3ee9989622d0d.tar.bz2
Fix Milter packet length calculation
git-svn-id: file:///svnroot/mailfromd/branches/release_3_1_patches@1356 7a8a7f39-df28-0410-adc6-e0d955640f24
Diffstat (more/less context) (ignore whitespace changes)
-rw-r--r--ChangeLog14
-rw-r--r--NEWS14
-rw-r--r--gacopyz/gacopyz.c10
-rw-r--r--src/engine.c9
-rw-r--r--src/main.c3
-rw-r--r--src/mu_dbm.c292
-rw-r--r--src/mu_dbm.h2
7 files changed, 241 insertions, 103 deletions
diff --git a/ChangeLog b/ChangeLog
index b1c7bf7..5b02def 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2007-04-16 Sergey Poznyakoff <gray@gnu.org.ua>
+
+ * src/mu_dbm.c, src/mu_dbm.h, src/engine.c, src/main.c: Fix
+ Berkeley DB locking.
+
+ * gacopyz/gacopyz.c (shan_connect): Fix packet length
+ calculation.
+
+ * NEWS: Update
+
+2007-04-12 Sergey Poznyakoff <gray@gnu.org.ua>
+
+ * src/mu_dbm.c: Undo CDB. Implement normal (sic!) locking.
+
2007-04-11 Sergey Poznyakoff <gray@gnu.org.ua>
* src/mu_dbm.c, src/mu_dbm.h (mu_dbm_finish, mu_dbm_strerror): New
diff --git a/NEWS b/NEWS
index 982df9a..fc5f2c6 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,4 @@
-Mailfromd NEWS -- history of user-visible changes. 2007-04-06
+Mailfromd NEWS -- history of user-visible changes. 2007-04-16
Copyright (C) 2005, 2006, 2007 Sergey Poznyakoff
See the end of file for copying conditions.
@@ -36,6 +36,18 @@ program state directory.
Bugfixes:
+** Fix incorrect packet length calculation in connect Milter handler.
+The bug manifested itself with the following log messages:
+
+- In mailfromd log:
+
+ MailfromFilter: shan_connect: wrong length
+
+- In Sendmail log:
+
+ milter_read(mailfrom): cmd read returned 0, expecting 5
+ Milter(mailfrom): to error state
+
* If, during sender verification, the remote server replies with
4xx to MAIL FROM command, do not try next sender address, but tempfail
immediately.
diff --git a/gacopyz/gacopyz.c b/gacopyz/gacopyz.c
index ae33794..d7f32b8 100644
--- a/gacopyz/gacopyz.c
+++ b/gacopyz/gacopyz.c
@@ -829,17 +829,11 @@ shan_connect(SMFICTX *ctx, union state_arg *arg, unsigned char *cmd)
s = arg->strings[1];
family = *s++;
- len = strlen(s);
if (family != SMFIA_UNKNOWN) {
- if (len < sizeof port) {
- gacopyz_log(ctx->conn, SMI_LOG_ERR,
- "%s: shan_connect: wrong length",
- ctx->conn->desc.xxfi_name);
- return sret_abort;
- }
port = *(unsigned short*) s;
s += sizeof port;
- len -= sizeof port;
+
+ len = strlen(s);
if (len > 0 && s[len - 1])
return sret_abort;
diff --git a/src/engine.c b/src/engine.c
index ff62333..5548dd8 100644
--- a/src/engine.c
+++ b/src/engine.c
@@ -1098,13 +1098,6 @@ child_start()
return 0;
}
-static int
-child_finish()
-{
- mu_dbm_finish();
- return 0;
-}
-
struct smfiDesc smfilter =
{
"MailfromFilter",
@@ -1124,7 +1117,7 @@ struct smfiDesc smfilter =
NULL, /* unknown command handler */
NULL, /* data handler */
child_start, /* child start */
- child_finish, /* child finish */
+ NULL, /* child finish */
NULL /* idle callback */
#endif
};
diff --git a/src/main.c b/src/main.c
index 14834f1..aec570a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -711,8 +711,7 @@ option_pidfile(char *opt, void **pval, char *newval)
mu_error("Invalid pidfile name: must be absolute");
return 1;
}
- *pval = newval;
- return 0;
+ return option_string(opt, pval, newval);
}
int
diff --git a/src/mu_dbm.c b/src/mu_dbm.c
index 6229106..86100cc 100644
--- a/src/mu_dbm.c
+++ b/src/mu_dbm.c
@@ -34,6 +34,10 @@
#include <mailutils/errno.h>
#include <mailfrom.h>
+#ifndef MU_ERR_EXISTS
+# define MU_ERR_EXISTS EEXIST
+#endif
+
int
mu_fcheck_perm (int fd, int mode)
{
@@ -42,15 +46,12 @@ mu_fcheck_perm (int fd, int mode)
if (fstat (fd, &st) == -1)
{
if (errno == ENOENT)
- return 0;
+ return ENOENT;
else
- return 1;
+ return MU_ERR_FAILURE;
}
if ((st.st_mode & 0777) != mode)
- {
- errno = MU_ERR_UNSAFE_PERMS;
- return 1;
- }
+ return errno = MU_ERR_UNSAFE_PERMS;
return 0;
}
@@ -64,25 +65,18 @@ mu_check_perm (const char *name, int mode)
if (stat (name, &st) == -1)
{
if (errno == ENOENT)
- return 0;
+ return ENOENT;
else
- return 1;
+ return MU_ERR_FAILURE;
}
if ((st.st_mode & 0777) != mode)
- {
- errno = MU_ERR_UNSAFE_PERMS;
- return 1;
- }
+ return errno = MU_ERR_UNSAFE_PERMS;
+
return 0;
}
#if defined(WITH_GDBM)
-void
-mu_dbm_finish ()
-{
-}
-
int
mu_dbm_open (char *name, DBM_FILE *db, int flags, int mode, int *ro)
{
@@ -95,7 +89,8 @@ mu_dbm_open (char *name, DBM_FILE *db, int flags, int mode, int *ro)
return MU_ERR_UNSAFE_PERMS;
if (ro)
- *ro = 0;
+ *ro = 0;
+
switch (flags)
{
case MU_STREAM_CREAT:
@@ -211,6 +206,8 @@ mu_dbm_datum_free (DBM_DATUM *datum)
const char *
mu_dbm_strerror ()
{
+ if (errno == MU_ERR_UNSAFE_PERMS)
+ return mu_strerror (errno);
return gdbm_strerror (gdbm_errno);
}
@@ -218,28 +215,126 @@ mu_dbm_strerror ()
int mu_dbm_errno;
-#if DB_VERSION_MAJOR >= 2
-static DB_ENV *dbenv;
-# if DB_VERSION_MAJOR == 2
-# define FREE_ENV() db_appexit (dbenv)
-# else
-# define FREE_ENV() \
- if (dbenv) \
- { \
- dbenv->remove (dbenv, mailfromd_state_dir, 0); \
- dbenv->close (dbenv, 0); \
- }
-# endif
-#else
-# define FREE_ENV()
-#endif
+static int
+try_lock (int fd, int type)
+{
+ int i;
+ struct flock fl;
+
+ memset (&fl, 0, sizeof fl);
+ fl.l_whence = SEEK_SET;
+ fl.l_type = type;
-void
-mu_dbm_finish ()
+ for (i = 0; i < lock_retry_count_option; i++)
+ {
+ if (fcntl (fd, F_SETLK, &fl) == 0)
+ return 0;
+ if (!(errno == EAGAIN || errno == EACCES))
+ break;
+ sleep (lock_retry_timeout_option);
+ }
+ return errno;
+}
+
+static int
+lock_file (const char *name, int fd, int type)
{
- FREE_ENV ();
+ int rc = 0;
+ pid_t holding_pid = 0;
+
+ rc = try_lock (fd, type);
+ if (rc)
+ {
+ struct flock fl;
+
+ fl.l_pid = 0;
+ if (fcntl (fd, F_GETLK, &fl) == 0)
+ {
+ if (fl.l_pid)
+ {
+ if (kill (fl.l_pid, 0) == 0)
+ mu_error ("%s is locked by process ID %lu",
+ name, (unsigned long) fl.l_pid);
+ else if (errno == ESRCH)
+ {
+ mu_error ("%s is locked by nonexisting process ID %lu; "
+ "retrying",
+ name, (unsigned long) fl.l_pid);
+ if (unlink (name))
+ mu_error ("cannot unlink file %s: %s",
+ name, mu_strerror (errno));
+ else
+ {
+ rc = try_lock (fd, type);
+ if (rc)
+ mu_error ("%s: cannot aquire lock: %s",
+ name, mu_strerror (rc));
+ }
+ }
+ else
+ mu_error ("%s seems locked by process ID %lu, "
+ "but its validity cannot be verified: %s",
+ name, (unsigned long) holding_pid,
+ mu_strerror (errno));
+ }
+ }
+ else
+ mu_error ("cannot obtain locker info for %s: %s",
+ name, mu_strerror (errno));
+ }
+ return rc;
}
+#define LOCK_SUFFIX ".lock"
+
+static int
+make_lockfile (char *name, int lktype, int *pfd, char **plockname)
+{
+ int rc;
+ int fd;
+ char *p, *lockname;
+ size_t size, namesize;
+
+ size = strlen (mailfromd_state_dir);
+ if (strlen (name) > size
+ && memcmp (name, mailfromd_state_dir, size) == 0
+ && name[size] == '/')
+ p = name + size + 1;
+ else
+ p = name;
+
+ namesize = size + 1 + strlen (p) + sizeof LOCK_SUFFIX;
+ lockname = malloc (namesize);
+ if (!lockname)
+ return errno;
+ strcpy (lockname, mailfromd_state_dir);
+ strcat (lockname, "/");
+ strcat (lockname, p);
+ strcat (lockname, LOCK_SUFFIX);
+ for (p = lockname + size + 1; *p; p++)
+ if (*p == '/')
+ *p = '-';
+
+ fd = open (lockname, O_CREAT|O_RDWR, 0600);
+ if (fd == -1)
+ {
+ free (lockname);
+ return errno;
+ }
+
+ rc = lock_file (lockname, fd, lktype);
+ if (rc)
+ {
+ free (lockname);
+ return rc;
+ }
+
+ *plockname = lockname;
+ *pfd = fd;
+ return 0;
+}
+
+
static void
mu_dbm_errcall_fcn (const char *errpfx, char *msg)
{
@@ -252,50 +347,18 @@ mu_dbm_errcall_fcn (const char *errpfx, char *msg)
int
mu_dbm_open (char *name, DBM_FILE *dbm, int flags, int mode, int *ro)
{
- int f, rc;
+ int f;
DB *db = NULL;
+ int fd;
+ char *lockname = NULL;
+ int oflags;
+ int lktype;
+
+ mu_dbm_errno = mu_check_perm (name, mode);
- if (mu_check_perm (name, mode))
+ if (mu_dbm_errno && mu_dbm_errno != ENOENT)
return MU_ERR_UNSAFE_PERMS;
-#if DB_VERSION_MAJOR >= 2
- if (!dbenv)
- {
-# if DB_VERSION_MAJOR == 2
- dbenv = calloc (1, sizeof dbenv[0]);
- if (!dbenv)
- {
- mu_dbm_errno = ENOMEM;
- return MU_ERR_FAILURE;
- }
- mu_dbm_errno = db_appinit (mailfromd_state_dir, NULL, dbenv,
- DB_CREATE | DB_INIT_LOCK | DB_INIT_MPOOL);
- if (mu_dbm_errno)
- return MU_ERR_FAILURE;
- dbenv->db_errcall = mu_dbm_errcall_fcn;
-
-# else
- if (mu_dbm_errno = db_env_create (&dbenv, 0))
- {
- mu_error ("Cannot create DB environment: %s",
- db_strerror (mu_dbm_errno));
- return MU_ERR_FAILURE;
- }
-
- dbenv->set_errcall (dbenv, mu_dbm_errcall_fcn);
-
- if (mu_dbm_errno = dbenv->open (dbenv, mailfromd_state_dir,
- DB_CREATE | DB_INIT_CDB | DB_INIT_MPOOL,
- 0600))
- {
- mu_error ("Cannot open DB environment: %s",
- db_strerror(mu_dbm_errno));
- return MU_ERR_FAILURE;
- }
-# endif /* DB_VERSION_MAJOR == 2 */
- }
-#endif /* DB_VERSION_MAJOR >= 2 */
-
if (ro)
*ro = 0;
@@ -303,25 +366,42 @@ mu_dbm_open (char *name, DBM_FILE *dbm, int flags, int mode, int *ro)
{
case MU_STREAM_CREAT:
f = DB_CREATE | DB_TRUNCATE;
+ lktype = F_WRLCK;
break;
case MU_STREAM_READ:
+ if (mu_dbm_errno == ENOENT)
+ return MU_ERR_FAILURE;
f = DB_RDONLY;
+ lktype = F_RDLCK;
break;
case MU_STREAM_RDWR:
f = DB_CREATE;
+ lktype = F_WRLCK;
break;
default:
- errno = EINVAL;
- return -1;
+ mu_dbm_errno = EINVAL;
+ return MU_ERR_FAILURE;
}
+#ifdef DB_FCNTL_LOCKING
+ f |= DB_FCNTL_LOCKING;
+#endif
+
+ mu_dbm_errno = make_lockfile (name, lktype, &dbm->lockfd, &dbm->lockname);
+ if (mu_dbm_errno)
+ {
+ mu_error ("Cannot lock file %s: %s",
+ name, mu_strerror (mu_dbm_errno));
+ return MU_ERR_FAILURE;
+ }
+
#if DB_VERSION_MAJOR == 2
- mu_dbm_errno = db_open (name, DB_HASH, f, mode, dbenv, NULL, &db);
+ mu_dbm_errno = db_open (name, DB_HASH, f, mode, NULL, NULL, &db);
#else
- mu_dbm_errno = db_create (&db, dbenv, 0);
+ mu_dbm_errno = db_create (&db, NULL, 0);
if (mu_dbm_errno != 0 || db == NULL)
return MU_ERR_FAILURE;
# if DB_VERSION_MAJOR == 3
@@ -331,11 +411,19 @@ mu_dbm_open (char *name, DBM_FILE *dbm, int flags, int mode, int *ro)
# endif
#endif
if (mu_dbm_errno)
- return MU_ERR_FAILURE;
+ {
+ unlink (dbm->lockname);
+ free (dbm->lockname);
+ close (dbm->lockfd);
+ return MU_ERR_FAILURE;
+ }
+
+ db->sync (db, 0);
dbm->name = strdup (name);
dbm->db = db;
dbm->dbc = NULL;
+
return 0;
}
@@ -349,9 +437,13 @@ mu_dbm_close (DBM_FILE *db)
if (mu_dbm_errno)
return MU_ERR_FAILURE;
}
+
+ db->db->sync (db->db, 0);
mu_dbm_errno = db->db->close (db->db, 0);
- if (mu_dbm_errno == DB_INCOMPLETE)
- mu_dbm_errno = 0;
+ unlink (db->lockname);
+ free (db->lockname);
+ close (db->lockfd);
+
free (db->name);
return (mu_dbm_errno == 0) ? 0 : MU_ERR_FAILURE;
}
@@ -468,7 +560,39 @@ mu_dbm_datum_free(DBM_DATUM *datum)
const char *
mu_dbm_strerror ()
{
- return db_strerror (mu_dbm_errno);
+ if (errno == MU_ERR_UNSAFE_PERMS)
+ return mu_strerror (errno);
+#if DB_VERSION_MAJOR == 2
+ switch (mu_dbm_errno)
+ {
+ case DB_INCOMPLETE:
+ return "Sync didn't finish";
+
+ case DB_KEYEMPTY:
+ return "The key/data pair was deleted or was never created by the user";
+ break;
+
+ case DB_KEYEXIST:
+ return "The key/data pair already exists";
+
+ case DB_LOCK_DEADLOCK:
+ return "Locker killed to resolve deadlock";
+
+ case DB_LOCK_NOTGRANTED:
+ return "Lock unavailable, no-wait set";
+
+ case DB_LOCK_NOTHELD:
+ return "Lock not held by locker";
+
+ case DB_NOTFOUND:
+ return "Key/data pair not found (EOF)";
+
+ default:
+ return "Unknown error";
+ }
+#else
+ return mu_strerror (mu_dbm_errno);
+#endif
}
#endif
diff --git a/src/mu_dbm.h b/src/mu_dbm.h
index e4742bb..8b49d28 100644
--- a/src/mu_dbm.h
+++ b/src/mu_dbm.h
@@ -41,6 +41,8 @@ typedef datum DBM_DATUM;
struct mu_db_file
{
char *name;
+ int lockfd;
+ char *lockname;
DB *db;
DBC *dbc;
};

Return to:

Send suggestions and report system problems to the System administrator.