Browse code

Fix file descriptor leak

git-svn: trunk@2445

Nigel Horne authored on 2006/10/29 00:57:49
Showing 2 changed files
... ...
@@ -1,3 +1,8 @@
1
+Sat Oct 28 16:56:51 BST 2006 (njh)
2
+----------------------------------
3
+  * clamav-milter:	Fix file descriptor leak when more than one email is
4
+				sent on a connection.
5
+
1 6
 Sat Oct 28 17:33:06 CEST 2006 (tk)
2 7
 ----------------------------------
3 8
   * libclamav/regex_list.c: .pdb/.wdb files now use colon as delimiter
... ...
@@ -40,7 +45,7 @@ Wed Oct 25 17:39:24 CEST 2006 (tk)
40 40
 Wed Oct 25 12:40:10 CEST 2006 (acab)
41 41
 ------------------------------------
42 42
   * clamscan/clamscan.c: fix typo breaking -l (closes bug#83)
43
-  			 reported by Yaniv Kaul <ykaul * zone.checkpoint.com>
43
+			 reported by Yaniv Kaul <ykaul * zone.checkpoint.com>
44 44
 
45 45
 Wed Oct 25 04:30:36 CEST 2006 (acab)
46 46
 ------------------------------------
... ...
@@ -23,9 +23,9 @@
23 23
  *
24 24
  * For installation instructions see the file INSTALL that came with this file
25 25
  */
26
-static	char	const	rcsid[] = "$Id: clamav-milter.c,v 1.291 2006/10/13 14:42:15 njh Exp $";
26
+static	char	const	rcsid[] = "$Id: clamav-milter.c,v 1.292 2006/10/28 15:56:23 njh Exp $";
27 27
 
28
-#define	CM_VERSION	"devel-131006"
28
+#define	CM_VERSION	"devel-281006"
29 29
 
30 30
 #if HAVE_CONFIG_H
31 31
 #include "clamav-config.h"
... ...
@@ -275,7 +275,7 @@ static	sfsistat	clamfi_eom(SMFICTX *ctx);
275 275
 static	sfsistat	clamfi_abort(SMFICTX *ctx);
276 276
 static	sfsistat	clamfi_close(SMFICTX *ctx);
277 277
 static	void		clamfi_cleanup(SMFICTX *ctx);
278
-static	void		clamfi_free(struct privdata *privdata);
278
+static	void		clamfi_free(struct privdata *privdata, int free);
279 279
 static	int		clamfi_send(struct privdata *privdata, size_t len, const char *format, ...);
280 280
 static	long		clamd_recv(int sock, char *buf, size_t len);
281 281
 static	off_t		updateSigFile(void);
... ...
@@ -299,6 +299,8 @@ static	void	timeoutBlacklist(char *ip_address, int time_of_blacklist);
299 299
 static	void	quit(void);
300 300
 static	void	broadcast(const char *mess);
301 301
 static	int	loadDatabase(void);
302
+static	int	increment_connections(void);
303
+static	void	decrement_connections(void);
302 304
 
303 305
 #ifdef	SESSION
304 306
 static	pthread_mutex_t	version_mutex = PTHREAD_MUTEX_INITIALIZER;
... ...
@@ -1437,6 +1439,15 @@ main(int argc, char **argv)
1437 1437
 			}
1438 1438
 
1439 1439
 #ifndef	SESSION
1440
+			if(serverIPs[i] == (int)inet_addr("127.0.0.1")) {
1441
+				/*
1442
+				 * Fudge to allow clamd to come up on
1443
+				 * our local machine
1444
+				 */
1445
+				sync();
1446
+				sleep(2);
1447
+			}
1448
+
1440 1449
 			if(pingServer(i))
1441 1450
 				activeServers++;
1442 1451
 			else {
... ...
@@ -2500,6 +2511,7 @@ clamfi_connect(SMFICTX *ctx, char *hostname, _SOCK_ADDR *hostaddr)
2500 2500
 	if(smfi_getpriv(ctx) != NULL) {
2501 2501
 		/* More than one connection command, "can't happen" */
2502 2502
 		cli_warnmsg("clamfi_connect: called more than once\n");
2503
+		clamfi_cleanup(ctx);
2503 2504
 		return cl_error;
2504 2505
 	}
2505 2506
 
... ...
@@ -2575,72 +2587,12 @@ clamfi_envfrom(SMFICTX *ctx, char **argv)
2575 2575
 			free(privdata);
2576 2576
 			return cl_error;
2577 2577
 		}
2578
-		if(max_children > 0) {
2579
-			int rc = 0;
2580
-
2581
-			pthread_mutex_lock(&n_children_mutex);
2582
-
2583
-			/*
2584
-			 * Wait a while since sendmail doesn't like it if we
2585
-			 * take too long replying. Effectively this means that
2586
-			 * max_children is more of a hint than a rule
2587
-			 */
2588
-			if(n_children >= max_children) {
2589
-				struct timespec timeout;
2590
-				struct timeval now;
2591
-				struct timezone tz;
2592
-
2593
-				logg((dont_wait) ?
2594
-						_("hit max-children limit (%u >= %u)\n") :
2595
-						_("hit max-children limit (%u >= %u): waiting for some to exit\n"),
2596
-					n_children, max_children);
2597
-
2598
-				if(dont_wait) {
2599
-					pthread_mutex_unlock(&n_children_mutex);
2600
-					smfi_setreply(ctx, "451", "4.3.2", _("AV system temporarily overloaded - please try later"));
2601
-					free(privdata);
2602
-					smfi_setpriv(ctx, NULL);
2603
-					return SMFIS_TEMPFAIL;
2604
-				}
2605
-				/*
2606
-				 * Wait for an amount of time for a child to go
2607
-				 *
2608
-				 * Use pthread_cond_timedwait rather than
2609
-				 * pthread_cond_wait since the sendmail which
2610
-				 * calls us will have a timeout that we don't
2611
-				 * want to exceed, stops sendmail getting
2612
-				 * fidgety.
2613
-				 *
2614
-				 * Patch from Damian Menscher
2615
-				 * <menscher@uiuc.edu> to ensure it wakes up
2616
-				 * when a child goes away
2617
-				 */
2618
-				gettimeofday(&now, &tz);
2619
-				do {
2620
-					logg(_("n_children %d: waiting %d seconds for some to exit"),
2621
-						n_children, child_timeout);
2622
-
2623
-					if(child_timeout == 0) {
2624
-						pthread_cond_wait(&n_children_cond, &n_children_mutex);
2625
-						rc = 0;
2626
-					} else {
2627
-						timeout.tv_sec = now.tv_sec + child_timeout;
2628
-						timeout.tv_nsec = 0;
2629
-
2630
-						rc = pthread_cond_timedwait(&n_children_cond, &n_children_mutex, &timeout);
2631
-					}
2632
-				} while((n_children >= max_children) && (rc != ETIMEDOUT));
2633
-				logg(_("Finished waiting, n_children = %d\n"), n_children);
2634
-			}
2635
-			n_children++;
2636
-
2637
-			cli_dbgmsg(">n_children = %d\n", n_children);
2638
-			pthread_mutex_unlock(&n_children_mutex);
2639
-
2640
-			if(child_timeout && (rc == ETIMEDOUT))
2641
-				logg(_("*Timeout waiting for a child to die\n"));
2578
+		if(!increment_connections()) {
2579
+			smfi_setreply(ctx, "451", "4.3.2", _("AV system temporarily overloaded - please try later"));
2580
+			free(privdata);
2581
+			smfi_setpriv(ctx, NULL);
2582
+			return SMFIS_TEMPFAIL;
2642 2583
 		}
2643
-
2644 2584
 	} else {
2645 2585
 		/* More than one message on this connection */
2646 2586
 		char ip[INET_ADDRSTRLEN];
... ...
@@ -2650,8 +2602,8 @@ clamfi_envfrom(SMFICTX *ctx, char **argv)
2650 2650
 			logg("Rejected email from blacklisted IP %s\n", ip);
2651 2651
 
2652 2652
 			/*
2653
-			 * TODO: Option to greylist rather than blacklist, by sending
2654
-			 *	a try again code
2653
+			 * TODO: Option to greylist rather than blacklist, by
2654
+			 *	sending	a try again code
2655 2655
 			 * TODO: state *which* virus
2656 2656
 			 */
2657 2657
 			smfi_setreply(ctx, "550", "5.7.1", _("Your IP is blacklisted because your machine is infected with a virus"));
... ...
@@ -2666,7 +2618,7 @@ clamfi_envfrom(SMFICTX *ctx, char **argv)
2666 2666
 
2667 2667
 			return SMFIS_REJECT;
2668 2668
 		}
2669
-		memset(privdata, '\0', sizeof(struct privdata));
2669
+		clamfi_free(privdata, 1);
2670 2670
 		strcpy(privdata->ip, ip);
2671 2671
 	}
2672 2672
 
... ...
@@ -3043,7 +2995,6 @@ clamfi_eom(SMFICTX *ctx)
3043 3043
 
3044 3044
 	if(!external) {
3045 3045
 		const char *virname;
3046
-		unsigned long int scanned = 0L;
3047 3046
 
3048 3047
 		pthread_mutex_lock(&root_mutex);
3049 3048
 		privdata->root = cl_dup(root);
... ...
@@ -3053,7 +3004,7 @@ clamfi_eom(SMFICTX *ctx)
3053 3053
 			clamfi_cleanup(ctx);
3054 3054
 			return cl_error;
3055 3055
 		}
3056
-		switch(cl_scanfile(privdata->filename, &virname, &scanned, privdata->root, &limits, options)) {
3056
+		switch(cl_scanfile(privdata->filename, &virname, NULL, privdata->root, &limits, options)) {
3057 3057
 			case CL_CLEAN:
3058 3058
 				if(logClean)
3059 3059
 					logg("#%s: OK", privdata->filename);
... ...
@@ -3650,7 +3601,6 @@ clamfi_eom(SMFICTX *ctx)
3650 3650
 			}
3651 3651
 		}
3652 3652
 	}
3653
-	/*clamfi_cleanup(ctx);*/
3654 3653
 
3655 3654
 	return rc;
3656 3655
 }
... ...
@@ -3666,6 +3616,7 @@ clamfi_abort(SMFICTX *ctx)
3666 3666
 	cli_dbgmsg("clamfi_abort\n");
3667 3667
 
3668 3668
 	clamfi_cleanup(ctx);
3669
+	decrement_connections();
3669 3670
 
3670 3671
 	cli_dbgmsg("clamfi_abort returns\n");
3671 3672
 
... ...
@@ -3678,6 +3629,7 @@ clamfi_close(SMFICTX *ctx)
3678 3678
 	logg("*clamfi_close\n");
3679 3679
 
3680 3680
 	clamfi_cleanup(ctx);
3681
+	decrement_connections();
3681 3682
 
3682 3683
 	return SMFIS_CONTINUE;
3683 3684
 }
... ...
@@ -3690,13 +3642,13 @@ clamfi_cleanup(SMFICTX *ctx)
3690 3690
 	cli_dbgmsg("clamfi_cleanup\n");
3691 3691
 
3692 3692
 	if(privdata) {
3693
-		clamfi_free(privdata);
3693
+		clamfi_free(privdata, 0);
3694 3694
 		smfi_setpriv(ctx, NULL);
3695 3695
 	}
3696 3696
 }
3697 3697
 
3698 3698
 static void
3699
-clamfi_free(struct privdata *privdata)
3699
+clamfi_free(struct privdata *privdata, int keep)
3700 3700
 {
3701 3701
 	cli_dbgmsg("clamfi_free\n");
3702 3702
 
... ...
@@ -3831,29 +3783,13 @@ clamfi_free(struct privdata *privdata)
3831 3831
 #endif
3832 3832
 		if(privdata->received)
3833 3833
 			free(privdata->received);
3834
-		free(privdata);
3835
-	}
3836 3834
 
3837
-	if(max_children > 0) {
3838
-		pthread_mutex_lock(&n_children_mutex);
3839
-		cli_dbgmsg("clamfi_free: n_children = %d\n", n_children);
3840
-		/*
3841
-		 * Deliberately errs on the side of broadcasting too many times
3842
-		 */
3843
-		if(n_children > 0)
3844
-			if(--n_children == 0) {
3845
-				cli_dbgmsg("%s is idle\n", progname);
3846
-				if(pthread_cond_broadcast(&watchdog_cond) < 0)
3847
-					perror("pthread_cond_broadcast");
3848
-			}
3849
-#ifdef	CL_DEBUG
3850
-		cli_dbgmsg("pthread_cond_broadcast\n");
3851
-#endif
3852
-		if(pthread_cond_broadcast(&n_children_cond) < 0)
3853
-			perror("pthread_cond_broadcast");
3854
-		cli_dbgmsg("<n_children = %d\n", n_children);
3855
-		pthread_mutex_unlock(&n_children_mutex);
3835
+		if(keep)
3836
+			memset(privdata, '\0', sizeof(struct privdata));
3837
+		else
3838
+			free(privdata);
3856 3839
 	}
3840
+
3857 3841
 	cli_dbgmsg("clamfi_free returns\n");
3858 3842
 }
3859 3843
 
... ...
@@ -4761,7 +4697,11 @@ move(const char *oldfile, const char *newfile)
4761 4761
 	ret = sendfile(out, in, &offset, statb.st_size);
4762 4762
 	close(in);
4763 4763
 	if(ret < 0) {
4764
-		/* fall back if sendfile fails, which shouldn't happen */
4764
+		/*
4765
+		 * Fall back if sendfile fails, which will happen on Linux
4766
+		 * 2.6 :-(. FreeBSD works correctly, so the ifdef should be
4767
+		 * fixed
4768
+		 */
4765 4769
 		close(out);
4766 4770
 		unlink(newfile);
4767 4771
 
... ...
@@ -5907,3 +5847,97 @@ useful_header(const char *cmd)
5907 5907
 
5908 5908
 	return 0;
5909 5909
 }
5910
+
5911
+static int
5912
+increment_connections(void)
5913
+{
5914
+	if(max_children > 0) {
5915
+		int rc = 0;
5916
+
5917
+		pthread_mutex_lock(&n_children_mutex);
5918
+
5919
+		/*
5920
+		 * Wait a while since sendmail doesn't like it if we
5921
+		 * take too long replying. Effectively this means that
5922
+		 * max_children is more of a hint than a rule
5923
+		 */
5924
+		if(n_children >= max_children) {
5925
+			struct timespec timeout;
5926
+			struct timeval now;
5927
+			struct timezone tz;
5928
+
5929
+			logg((dont_wait) ?
5930
+					_("hit max-children limit (%u >= %u)\n") :
5931
+					_("hit max-children limit (%u >= %u): waiting for some to exit\n"),
5932
+				n_children, max_children);
5933
+
5934
+			if(dont_wait) {
5935
+				pthread_mutex_unlock(&n_children_mutex);
5936
+				return 0;
5937
+			}
5938
+			/*
5939
+			 * Wait for an amount of time for a child to go
5940
+			 *
5941
+			 * Use pthread_cond_timedwait rather than
5942
+			 * pthread_cond_wait since the sendmail which
5943
+			 * calls us will have a timeout that we don't
5944
+			 * want to exceed, stops sendmail getting
5945
+			 * fidgety.
5946
+			 *
5947
+			 * Patch from Damian Menscher
5948
+			 * <menscher@uiuc.edu> to ensure it wakes up
5949
+			 * when a child goes away
5950
+			 */
5951
+			gettimeofday(&now, &tz);
5952
+			do {
5953
+				logg(_("n_children %d: waiting %d seconds for some to exit"),
5954
+					n_children, child_timeout);
5955
+
5956
+				if(child_timeout == 0) {
5957
+					pthread_cond_wait(&n_children_cond, &n_children_mutex);
5958
+					rc = 0;
5959
+				} else {
5960
+					timeout.tv_sec = now.tv_sec + child_timeout;
5961
+					timeout.tv_nsec = 0;
5962
+
5963
+					rc = pthread_cond_timedwait(&n_children_cond, &n_children_mutex, &timeout);
5964
+				}
5965
+			} while((n_children >= max_children) && (rc != ETIMEDOUT));
5966
+			logg(_("Finished waiting, n_children = %d\n"), n_children);
5967
+		}
5968
+		n_children++;
5969
+
5970
+		cli_dbgmsg(">n_children = %d\n", n_children);
5971
+		pthread_mutex_unlock(&n_children_mutex);
5972
+
5973
+		if(child_timeout && (rc == ETIMEDOUT))
5974
+			logg(_("*Timeout waiting for a child to die\n"));
5975
+	}
5976
+
5977
+	return 1;
5978
+}
5979
+
5980
+static void
5981
+decrement_connections(void)
5982
+{
5983
+	if(max_children > 0) {
5984
+		pthread_mutex_lock(&n_children_mutex);
5985
+		cli_dbgmsg("decrement_connections: n_children = %d\n", n_children);
5986
+		/*
5987
+		 * Deliberately errs on the side of broadcasting too many times
5988
+		 */
5989
+		if(n_children > 0)
5990
+			if(--n_children == 0) {
5991
+				cli_dbgmsg("%s is idle\n", progname);
5992
+				if(pthread_cond_broadcast(&watchdog_cond) < 0)
5993
+					perror("pthread_cond_broadcast");
5994
+			}
5995
+#ifdef	CL_DEBUG
5996
+		cli_dbgmsg("pthread_cond_broadcast\n");
5997
+#endif
5998
+		if(pthread_cond_broadcast(&n_children_cond) < 0)
5999
+			perror("pthread_cond_broadcast");
6000
+		cli_dbgmsg("<n_children = %d\n", n_children);
6001
+		pthread_mutex_unlock(&n_children_mutex);
6002
+	}
6003
+}