Re: dma(8): Fix race condition in multi-recipient delivery

["Simon 'corecode' Schubert" <corecode@xxxxxxxxxxxx>]

Daniel Roethlisberger wrote:
> Simon 'corecode' Schuber <corecode@fs.ei.tum.de> 2009-07-09:
> > --- a/libexec/dma/dma.c
> > +++ b/libexec/dma/dma.c
> > @@ -484,6 +484,7 @@ go_background(struct queue *queue)
> >  {
> >  	struct sigaction sa;
> >  	struct qitem *it;
> > +	FILE *newqf;
> >  	pid_t pid;
> >  
> >  	if (daemonize && daemon(0, 0) != 0) {
> > @@ -515,6 +516,17 @@ go_background(struct queue *queue)
> >  			 *
> >  			 * return and deliver mail
> >  			 */
> > +			/*
> > +			 * We have to prevent sharing of fds between children, so
> > +			 * we have to dup the queue fd.
> > +			 */
> > +			newqf = fdopen(fileno(it->queuef), "r");
>
> Actually, fdopen() will not dup() the file descriptors, just
> create a new FILE* handle operating on the same fd.

Uh.  I ment to add a dup() there, but I moved a lot of code, so I 
forgot...  What about the attached patch?

cheers
  simon

--
  <3 the future  +++  RENT this banner advert  +++   ASCII Ribbon   /"\
  rock the past  +++  space for low CHF NOW!1  +++     Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \

From d8f2721284e590f49a424bc58cd57c86a18be8bc Mon Sep 17 00:00:00 2001
From: Simon Schubert <simon.schubert@epfl.ch>
Date: Thu, 9 Jul 2009 23:24:35 +0200
Subject: [PATCH] dma: prevent races from sharing fd between children

On fork, fds are shared between children.  If two processes work on
different recipients, but on the same queue file, they might get
confused when the fd (and thus the offset) is shared.  Prevent this by
re-opening the fd after fork.

Reported-by: Daniel Roethlisberger <daniel@roe.ch>
---
 libexec/dma/dma.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/libexec/dma/dma.c b/libexec/dma/dma.c
index 4633b0c..793e327 100644
--- a/libexec/dma/dma.c
+++ b/libexec/dma/dma.c
@@ -484,6 +484,7 @@ go_background(struct queue *queue)
 {
 	struct sigaction sa;
 	struct qitem *it;
+	FILE *newqf;
 	pid_t pid;
 
 	if (daemonize && daemon(0, 0) != 0) {
@@ -515,6 +516,17 @@ go_background(struct queue *queue)
 			 *
 			 * return and deliver mail
 			 */
+			/*
+			 * We have to prevent sharing of fds between children, so
+			 * we have to dup the queue fd.
+			 */
+			newqf = fdopen(fileno(it->queuef), "r");
+			if (newqf == NULL) {
+				syslog(LOG_ERR, "can not dup queue fd: %m");
+				exit(1);
+			}
+			fclose(it->queuef);
+			it->queuef = newqf;
 			return (it);
 
 		default:
-- 
1.6.2.149.g6462