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

[Daniel Roethlisberger <daniel@xxxxxx>]

Simon 'corecode' Schuber <corecode@fs.ei.tum.de> 2009-07-10:
> Simon 'corecode' Schubert wrote:
> >Uh.  I ment to add a dup() there, but I moved a lot of code, so I 
> >forgot...  What about the attached patch?
[...]
> --- 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(dup(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:

This is still not sufficient (logs below; of 3 recipients, 2 were
successful but with corrupted contents, 1 failed due to queue
file corruption).

To go down that route, I think you'd need to fully reopen the
file starting with open().  This opens another can of worms:
unlink() races.  I guess it is easier to lock the queue file
instead of fully reopening and fixing the unlink() race.  But
maybe I'm missing something.

Jul 10 10:41:30 marvin dma[52950]: d6.28401130: mail from=<roe@marvin.ustcor.roe.ch> to=<daniel+3@roe.ch>
Jul 10 10:41:30 marvin dma[52951]: d6.28401100: mail from=<roe@marvin.ustcor.roe.ch> to=<daniel+2@roe.ch>
Jul 10 10:41:30 marvin dma[52951]: d6.28401100: trying delivery
Jul 10 10:41:30 marvin dma[52951]: d6.28401100: using smarthost (calvin.ustdmz.roe.ch:587)
Jul 10 10:41:30 marvin dma[52949]: d6.284010d0: mail from=<roe@marvin.ustcor.roe.ch> to=<daniel+1@roe.ch>
Jul 10 10:41:30 marvin dma[52949]: d6.284010d0: trying delivery
Jul 10 10:41:30 marvin dma[52949]: d6.284010d0: using smarthost (calvin.ustdmz.roe.ch:587)
Jul 10 10:41:30 marvin dma[52950]: d6.28401130: trying delivery
Jul 10 10:41:30 marvin dma[52950]: d6.28401130: using smarthost (calvin.ustdmz.roe.ch:587)
Jul 10 10:41:34 marvin dma[52951]: d6.28401100: SSL initialization successful
Jul 10 10:41:34 marvin dma[52951]: d6.28401100: Use SMTP authentication
Jul 10 10:41:37 marvin dma[52950]: d6.28401130: SSL initialization successful
Jul 10 10:41:37 marvin dma[52949]: d6.284010d0: SSL initialization successful
Jul 10 10:41:37 marvin dma[52950]: d6.28401130: Use SMTP authentication
Jul 10 10:41:37 marvin dma[52949]: d6.284010d0: Use SMTP authentication
Jul 10 10:41:45 marvin dma[52951]: d6.28401100: delivery successful
Jul 10 10:41:48 marvin dma[52949]: d6.284010d0: remote delivery failed:corrupted queue file
Jul 10 10:41:48 marvin dma[52949]: d6.284010d0: delivery failed, bouncing
Jul 10 10:41:48 marvin dma[52949]: d8.28401250: mail from=<> to=<roe>
Jul 10 10:41:48 marvin dma[52949]: d8.28401250: trying delivery
Jul 10 10:41:48 marvin dma[52949]: d8.28401250: delivery successful
Jul 10 10:41:57 marvin dma[52950]: d6.28401130: delivery successful

-- 
Daniel Roethlisberger
http://daniel.roe.ch/