[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: POP3d locking
|
From: |
Sergey Poznyakoff |
|
Subject: |
Re: POP3d locking |
|
Date: |
Sat, 28 Apr 2001 12:45:14 +0300 |
Hello,
I have finished the locking version of pop3d. The version has been
working on my mail server since thursday and behaves itself well.
I have followed the basic guidelines we have discussed, except
that I have not implemented locker_set_??? functions. Instead,
I have modified locker.c to implement reference count. The
struct _locker is now:
struct _locker
{
int fd;
int refcnt;
char *fname;
int flags;
};
The refcnt member gets incremented each time locker_lock is called and
decremented each time locker_unlock is called. Actual locking is
performed only if refcnt == 0 on entry to locker_lock. Similarly,
unlocking is performed only when refcnt gets decremented to 0 by
locker_unlock. Also I have added basic support for NFS-secure
locking.
The program tries to touch its lock each LOCK_EXPIRE_TIME/2 seconds.
In order to make it possible I had to move LOCK_EXPIRE_TIME declaration
to header file locker.h. (btw: Maybe we should made it configurable?)
Another approach would be to touch lockfile after receiving
a command from the user, but it seems to have considerable drawbacks:
on slow links retrieving a letter could take considerable time, which
is bigger that LOCK_EXPIRE_TIME. Then, while one session is still
retrieving the message, another user could open the same mailbox,
the program would decide that it has expired and destroy it. Thus,
all modifications done by the first session would be lost. So, I have
decided to touch the lockfile in regular intervals. What do you
think?
The select() or read() calls in pop3d_readline() can now be
interrupted by SIGALRM, so appropriate handling for EINTR is
added to the function.
Just in case, while in transaction state, a consistency check is
performed after receiving each command, so if the mailbox gets
updated by any program that does not honour the lock, the modifications
will not be flushed. (Addition of newly arrived messages is handled
correctly, though).
Some additional changes that were made:
1. locker_lock while trying to read pid value from the pidfile
passed wrong buffer length to read() call, namely:
read(fd, buf, sizeof (pid_t))
whereas buf was declared as buf[16]. Consequently, most of the
time the process was considered dead, unless his pid consisted
of three decimal digits.
2. Return values of all signal handlers are changed to RETSIGTYPE
and configure.in is modified accordingly. This is more portable.
3. Current implementation of pop3d_signal would cause main process
to attempt to perform fprintf on NULL file pointer (ofile). Sure,
the main process should not crash, but one never knows... So I
have modified pop3d_signal to handle this appropriately.
Additionally, revising uidl.c I noticed that uild buffer is declared
as char uidl[128]. RFC sais it is 70 octets + newline character.
Should we be more RFC compliant?
Well, for the time being, that is all. The patch follows.
Cheers,
Sergey Poznyakoff
---------------------------------------------------------------------------
... on n'est jamais heureux la ou l'on est...
Index: configure.in
===================================================================
RCS file: /cvs/mailutils/configure.in,v
retrieving revision 1.43
diff -c -b -w -r1.43 configure.in
*** configure.in 2001/04/24 05:09:26 1.43
--- configure.in 2001/04/28 08:47:10
***************
*** 66,71 ****
--- 66,72 ----
AC_TYPE_OFF_T
AC_TYPE_PID_T
AC_TYPE_SIZE_T
+ AC_TYPE_SIGNAL
dnl Check for working functions
Index: include/mailutils/locker.h
===================================================================
RCS file: /cvs/mailutils/include/mailutils/locker.h,v
retrieving revision 1.2
diff -c -b -w -r1.2 locker.h
*** include/mailutils/locker.h 2000/10/24 04:35:34 1.2
--- include/mailutils/locker.h 2001/04/28 08:47:12
***************
*** 47,52 ****
--- 47,54 ----
#define MU_LOCKER_FCNTL 2
#define MU_LOCKER_TIME 4
+ #define LOCK_EXPIRE_TIME (5 * 60)
+
extern int locker_lock __P ((locker_t, int flag));
extern int locker_touchlock __P ((locker_t));
extern int locker_unlock __P ((locker_t));
Index: mailbox/locker.c
===================================================================
RCS file: /cvs/mailutils/mailbox/locker.c,v
retrieving revision 1.8
diff -c -b -w -r1.8 locker.c
*** mailbox/locker.c 2001/01/16 06:14:52 1.8
--- mailbox/locker.c 2001/04/28 08:47:12
***************
*** 35,47 ****
#include <mailutils/locker.h>
#define LOCKFILE_ATTR 0444
- #define LOCK_EXPIRE_TIME (5 * 60)
/* First draft by Brian Edmond. */
struct _locker
{
int fd;
char *fname;
int flags;
};
--- 35,47 ----
#include <mailutils/locker.h>
#define LOCKFILE_ATTR 0444
/* First draft by Brian Edmond. */
struct _locker
{
int fd;
+ int refcnt;
char *fname;
int flags;
};
***************
*** 75,80 ****
--- 75,81 ----
else
l->flags = MU_LOCKER_TIME; /* Default is time lock implementation. */
l->fd = -1;
+ l->refcnt = 0;
*plocker = l;
return 0;
}
***************
*** 102,107 ****
--- 103,115 ----
if (lock == NULL)
return EINVAL;
+ /* is the lock already applied?
+ FIXME: should we check flags != lock->flags ?? */
+ if (lock->fd != -1) {
+ lock->refcnt++;
+ return 0;
+ }
+
/*
Check for lock existance:
if it exists but the process is gone the lock can be removed,
***************
*** 111,117 ****
/* Check to see if this process is still running. */
if (lock->flags & MU_LOCKER_PID)
{
! if (read(fd, buf, sizeof (pid_t)) > 0)
{
if ((pid = atoi(buf)) > 0)
{
--- 119,125 ----
/* Check to see if this process is still running. */
if (lock->flags & MU_LOCKER_PID)
{
! if (read(fd, buf, sizeof (buf)) > 0)
{
if ((pid = atoi(buf)) > 0)
{
***************
*** 141,146 ****
--- 149,172 ----
if ((fd = open(lock->fname,
O_WRONLY | O_CREAT | O_EXCL, LOCKFILE_ATTR)) == -1)
return errno;
+ else
+ {
+ struct stat fn_stat;
+ struct stat fd_stat;
+
+ if (lstat(lock->fname, &fn_stat) ||
+ fstat(fd, &fd_stat) ||
+ fn_stat.st_nlink != 1 ||
+ fn_stat.st_dev != fd_stat.st_dev ||
+ fn_stat.st_ino != fd_stat.st_ino ||
+ fn_stat.st_uid != fd_stat.st_uid ||
+ fn_stat.st_gid != fd_stat.st_gid) {
+ close(fd);
+ unlink(lock->fname);
+ return EPERM;
+ }
+ }
+
/* Success. */
sprintf(buf, "%ld", (long)getpid());
write(fd, buf, strlen(buf));
***************
*** 163,168 ****
--- 189,196 ----
}
lock->fd = fd;
+ lock->refcnt++;
+
return 0;
}
***************
*** 177,185 ****
int
locker_unlock (locker_t lock)
{
! if (!lock || ! lock->fname || (lock->fd == -1))
return EINVAL;
!
if (lock->flags & MU_LOCKER_FCNTL)
{
struct flock fl;
--- 205,214 ----
int
locker_unlock (locker_t lock)
{
! if (!lock || ! lock->fname || (lock->fd == -1) || lock->refcnt <= 0)
return EINVAL;
! if (--lock->refcnt > 0)
! return 0;
if (lock->flags & MU_LOCKER_FCNTL)
{
struct flock fl;
***************
*** 195,197 ****
--- 224,228 ----
unlink(lock->fname);
return 0;
}
+
+
Index: pop3d/extra.c
===================================================================
RCS file: /cvs/mailutils/pop3d/extra.c,v
retrieving revision 1.11
diff -c -b -w -r1.11 extra.c
*** pop3d/extra.c 2001/04/24 05:09:27 1.11
--- pop3d/extra.c 2001/04/28 08:47:16
***************
*** 86,91 ****
--- 86,92 ----
int
pop3d_abquit (int reason)
{
+ pop3d_unlock();
mailbox_close (mbox);
mailbox_destroy (&mbox);
***************
*** 132,137 ****
--- 133,143 ----
if (ofile)
fflush (ofile);
closelog();
+
+ /*temporary hack by gray*/
+ chdir("/tmp");
+ abort();
+
exit (1);
}
***************
*** 158,167 ****
/* Default signal handler to call the pop3d_abquit() function */
! void
pop3d_signal (int signo)
{
! (void)signo;
syslog (LOG_CRIT, "got signal %d", signo);
pop3d_abquit (ERR_SIGNAL);
}
--- 164,183 ----
/* Default signal handler to call the pop3d_abquit() function */
! RETSIGTYPE
pop3d_signal (int signo)
+ {
+ syslog(LOG_CRIT, "got signal %d", signo);
+ if (!ofile)
{
! /* Master process */
! syslog(LOG_CRIT, "MASTER: exiting on signal");
! abort();
! }
! else
! {
! pop3d_abquit (ERR_SIGNAL);
! }
syslog (LOG_CRIT, "got signal %d", signo);
pop3d_abquit (ERR_SIGNAL);
}
***************
*** 187,200 ****
{
if (timeout > 0)
{
available = select (fd + 1, &rfds, NULL, NULL, &tv);
if (!available)
pop3d_abquit (ERR_TIMEOUT);
}
nread = read (fd, buf, sizeof (buf) - 1);
! if (nread < 1)
pop3d_abquit (ERR_DEAD_SOCK);
buf[nread] = '\0';
ret = realloc (ret, (total + nread + 1) * sizeof (char));
--- 203,225 ----
{
if (timeout > 0)
{
+ nread = 0;
available = select (fd + 1, &rfds, NULL, NULL, &tv);
if (!available)
pop3d_abquit (ERR_TIMEOUT);
+ else if (available == -1) {
+ if (errno == EINTR)
+ continue;
+ pop3d_abquit (ERR_DEAD_SOCK);
+ }
}
nread = read (fd, buf, sizeof (buf) - 1);
! if (nread < 1) {
! if (errno == EINTR)
! continue;
pop3d_abquit (ERR_DEAD_SOCK);
+ }
buf[nread] = '\0';
ret = realloc (ret, (total + nread + 1) * sizeof (char));
***************
*** 203,209 ****
memcpy (ret + total, buf, nread + 1);
total += nread;
}
! while (memchr (buf, '\n', nread) == NULL);
return ret;
}
--- 228,306 ----
memcpy (ret + total, buf, nread + 1);
total += nread;
}
! while (nread < 1 || memchr (buf, '\n', nread) == NULL);
return ret;
+ }
+
+ RETSIGTYPE
+ pop3d_sigalrm(int sig)
+ {
+ (void)sig;
+ syslog(LOG_DEBUG, "touching lock");
+ pop3d_touchlock();
+ #ifndef HAVE_SIGACTION
+ signal(sig, pop3d_sigalrm);
+ #endif
+ alarm(LOCK_EXPIRE_TIME/2);
+ }
+
+ int
+ pop3d_lock()
+ {
+ url_t url;
+ locker_t lock;
+ char *name;
+ int rc;
+
+ mailbox_get_url (mbox, &url);
+ name = url_to_string(url);
+ if (rc = locker_create(&lock, name, strlen(name),
+ MU_LOCKER_PID|MU_LOCKER_TIME))
+ {
+ syslog(LOG_NOTICE, "can't create mailbox '%s': %m", name);
+ return errno;
+ }
+
+ if (locker_lock(lock, MU_LOCKER_PID|MU_LOCKER_TIME))
+ {
+ syslog(LOG_NOTICE, "mailbox '%s' locked by another session",
+ name);
+ return ERR_MBOX_LOCK;
+ }
+ mailbox_set_locker(mbox, lock);
+
+ #ifdef HAVE_SIGACTION
+ {
+ struct sigaction act;
+ act.sa_handler = pop3d_sigalrm;
+ sigemptyset (&act.sa_mask);
+ act.sa_flags = 0;
+ sigaction (SIGALRM, &act, NULL);
+ }
+ #else
+ signal(SIGALRM, pop3d_sigalrm);
+ #endif
+
+ alarm(LOCK_EXPIRE_TIME/2);
+ return 0;
+ }
+
+ int
+ pop3d_touchlock()
+ {
+ locker_t lock;
+
+ mailbox_get_locker(mbox, &lock);
+ locker_touchlock(lock);
+ }
+
+ int
+ pop3d_unlock()
+ {
+ locker_t lock;
+
+ mailbox_get_locker(mbox, &lock);
+ if (lock)
+ locker_unlock(lock);
}
Index: pop3d/pop3d.c
===================================================================
RCS file: /cvs/mailutils/pop3d/pop3d.c,v
retrieving revision 1.26
diff -c -b -w -r1.26 pop3d.c
*** pop3d/pop3d.c 2001/04/24 07:20:29 1.26
--- pop3d/pop3d.c 2001/04/28 08:47:16
***************
*** 51,56 ****
--- 51,58 ----
static int syslog_error_printer __P ((const char *fmt, va_list ap));
+ #define DEFMAXCHILDREN 10 /* Default maximum number of children */
+
int
main (int argc, char **argv)
{
***************
*** 70,78 ****
{
case 'd':
mode = DAEMON;
! maxchildren = optarg ? strtoul (optarg, NULL, 10) : 10;
if (maxchildren <= 0)
! maxchildren = 10;
break;
case 'h':
--- 72,80 ----
{
case 'd':
mode = DAEMON;
! maxchildren = optarg ? strtoul (optarg, NULL, 10) : DEFMAXCHILDREN;
if (maxchildren <= 0)
! maxchildren = DEFMAXCHILDREN;
break;
case 'h':
***************
*** 219,224 ****
--- 221,227 ----
char *buf, *arg, *cmd;
struct hostent *htbuf;
char *local_hostname;
+ size_t total_count = 0;
ifile = infile;
ofile = fdopen (outfile, "w");
***************
*** 268,287 ****
{
fflush (ofile);
status = OK;
buf = pop3d_readline (ifile);
cmd = pop3d_cmd (buf);
arg = pop3d_args (buf);
! if (state == TRANSACTION && !mailbox_is_updated (mbox))
{
! static off_t mailbox_size;
! off_t newsize = 0;
! mailbox_get_size (mbox, &newsize);
! /* Did we shrink? */
! if (!mailbox_size)
! mailbox_size = newsize;
! else if (newsize < mailbox_size)
pop3d_abquit (ERR_MBOX_SYNC);
}
if (strlen (arg) > POP_MAXCMDLEN || strlen (cmd) > POP_MAXCMDLEN)
--- 271,289 ----
{
fflush (ofile);
status = OK;
+
buf = pop3d_readline (ifile);
cmd = pop3d_cmd (buf);
arg = pop3d_args (buf);
! if (state == TRANSACTION)
{
! size_t mesg_count;
!
! mailbox_messages_count(mbox, &mesg_count);
! if (total_count && mesg_count < total_count)
pop3d_abquit(ERR_MBOX_SYNC);
+ total_count = mesg_count;
}
if (strlen (arg) > POP_MAXCMDLEN || strlen (cmd) > POP_MAXCMDLEN)
***************
*** 394,403 ****
{
if (children > maxchildren)
{
! syslog (LOG_ERR, "too many children");
pause ();
continue;
}
connfd = accept (listenfd, (struct sockaddr *)&client, &size);
if (connfd == -1)
{
--- 396,406 ----
{
if (children > maxchildren)
{
! syslog(LOG_ERR, "too many children (%d)", children);
pause ();
continue;
}
+
connfd = accept (listenfd, (struct sockaddr *)&client, &size);
if (connfd == -1)
{
Index: pop3d/pop3d.h
===================================================================
RCS file: /cvs/mailutils/pop3d/pop3d.h,v
retrieving revision 1.17
diff -c -b -w -r1.17 pop3d.h
*** pop3d/pop3d.h 2001/04/24 07:20:29 1.17
--- pop3d/pop3d.h 2001/04/28 08:47:16
***************
*** 200,207 ****
extern int pop3d_mainloop __P ((int infile, int outfile));
extern void pop3d_daemon __P ((unsigned int maxchildren));
extern void pop3d_usage __P ((char *argv0));
! extern void pop3d_signal __P ((int));
! extern void pop3d_sigchld __P ((int));
extern void pop3d_daemon_init __P ((void));
extern char *pop3d_apopuser __P ((const char *user));
extern char *pop3d_readline __P ((int fd));
--- 200,207 ----
extern int pop3d_mainloop __P ((int infile, int outfile));
extern void pop3d_daemon __P ((unsigned int maxchildren));
extern void pop3d_usage __P ((char *argv0));
! extern RETSIGTYPE pop3d_signal __P ((int));
! extern RETSIGTYPE pop3d_sigchld __P ((int));
extern void pop3d_daemon_init __P ((void));
extern char *pop3d_apopuser __P ((const char *user));
extern char *pop3d_readline __P ((int fd));
Index: pop3d/quit.c
===================================================================
RCS file: /cvs/mailutils/pop3d/quit.c,v
retrieving revision 1.7
diff -c -b -w -r1.7 quit.c
*** pop3d/quit.c 2001/04/24 05:27:14 1.7
--- pop3d/quit.c 2001/04/28 08:47:16
***************
*** 32,37 ****
--- 32,38 ----
if (state == TRANSACTION)
{
+ pop3d_unlock();
if (mailbox_expunge (mbox) != 0)
err = ERR_FILE;
if (mailbox_close (mbox) != 0)
Index: pop3d/signal.c
===================================================================
RCS file: /cvs/mailutils/pop3d/signal.c,v
retrieving revision 1.5
diff -c -b -w -r1.5 signal.c
*** pop3d/signal.c 2001/04/24 05:27:14 1.5
--- pop3d/signal.c 2001/04/28 08:47:16
***************
*** 18,32 ****
#include "pop3d.h"
! void
pop3d_sigchld (int signo)
{
pid_t pid;
int status;
- int old_errno = errno ;
- (void)signo;
- errno = 0;
while ( (pid = waitpid(-1, &status, WNOHANG)) > 0)
--children;
#ifndef HAVE_SIGACTION
--- 18,29 ----
#include "pop3d.h"
! RETSIGTYPE
pop3d_sigchld (int signo)
{
pid_t pid;
int status;
while ( (pid = waitpid(-1, &status, WNOHANG)) > 0)
--children;
#ifndef HAVE_SIGACTION
***************
*** 34,38 ****
has to be rearm. */
signal (SIGCHLD, pop3d_sigchld);
#endif
- errno = old_errno;
}
--- 31,35 ----
has to be rearm. */
signal (SIGCHLD, pop3d_sigchld);
#endif
}
+
Index: pop3d/user.c
===================================================================
RCS file: /cvs/mailutils/pop3d/user.c,v
retrieving revision 1.12
diff -c -b -w -r1.12 user.c
*** pop3d/user.c 2001/04/24 05:27:14 1.12
--- pop3d/user.c 2001/04/28 08:47:16
***************
*** 82,87 ****
--- 82,88 ----
char *buf, pass[POP_MAXCMDLEN], *tmp, *cmd;
struct passwd *pw;
int status;
+ int lockit = 1;
#ifdef USE_LIBPAM
pam_handle_t *pamh;
int pamerror;
***************
*** 191,196 ****
--- 192,198 ----
state = AUTHORIZATION;
return ERR_UNKNOWN;
}
+ lockit = 0; /* do not attempt to lock /dev/null ! */
}
else
{
***************
*** 207,218 ****
if (pw != NULL && pw->pw_uid > 1)
setuid (pw->pw_uid);
fprintf (ofile, "+OK opened mailbox for %s\r\n", username);
/* mailbox name */
{
url_t url = NULL;
mailbox_get_url (mbox, &url);
! syslog (LOG_INFO, "User '%s' logged in with mailbox '%s'",
username, url_to_string (url));
}
return OK;
--- 209,230 ----
if (pw != NULL && pw->pw_uid > 1)
setuid (pw->pw_uid);
+ if (lockit && pop3d_lock())
+ {
+ mailbox_close(mbox);
+ mailbox_destroy(&mbox);
+ state = AUTHORIZATION;
+ return ERR_MBOX_LOCK;
+ }
+
fprintf (ofile, "+OK opened mailbox for %s\r\n", username);
/* mailbox name */
{
url_t url = NULL;
+ size_t total;
mailbox_get_url (mbox, &url);
! mailbox_messages_count (mbox, &total);
! syslog (LOG_INFO, "User '%s' logged in with mailbox '%s' (%d messages)",
username, url_to_string (url));
}
return OK;
----------------------------END OF PATCH---------------------------------
- Re: POP3d locking,
Sergey Poznyakoff <=