Hi guys,
This is my fourth submission for an environment variable patch for VirusEvents. The other 3 I submitted all had various bugs.
This patch makes the virusaction function reentrant (my previous patches broke reentrancy).
It didn't occur to me initially that modifying the environment is incompatible with reentrancy. As such, virusaction() needs to fork before calling putenv() and then calling system() (which itself forks, of course).
Please accept this patch, and use it to replace all my other previous patches.. thanks!!
PS: Could you kindly advise on whether this patch is rejected or accepted? Thanks..
-Calin
diff -ur clamav-devel/clamd/clamuko.c clamav-devel-modified/clamd/clamuko.c
--- clamav-devel/clamd/clamuko.c 2005-03-09 12:32:41.000000000 -0500
+++ clamav-devel-modified/clamd/clamuko.c 2005-03-14 10:08:30.000000000
-0500
@@ -173,7 +173,7 @@
if(scan && cl_scanfile(acc->filename, &virname, NULL, tharg->root,
tharg->limits, tharg->options) == CL_VIRUS) {
logg("Clamuko: %s: %s FOUND\n", acc->filename, virname);
- virusaction(virname, tharg->copt);
+ virusaction(acc->filename, virname, tharg->copt);
acc->deny = 1;
} else
acc->deny = 0;
diff -ur clamav-devel/clamd/others.c clamav-devel-modified/clamd/others.c
--- clamav-devel/clamd/others.c 2005-03-09 12:32:41.000000000 -0500
+++ clamav-devel-modified/clamd/others.c 2005-03-14 10:57:55.000000000
-0500
@@ -69,32 +69,98 @@
#include "cfgparser.h"
#include "session.h"
-void virusaction(const char *virname, const struct cfgstruct *copt)
+#define ENV_PREFIX "CLAM_VIRUSEVENT_"
+#define ENV_FILENAME (ENV_PREFIX "FILENAME")
+#define ENV_VIRUSNAME (ENV_PREFIX "VIRUSNAME")
+enum env_var { FILENAME = 0, VIRUSNAME, N_ENV_VARS };
+struct env_var_desc {
+ const char * name;
+ int namelen;
+};
+
+static const struct env_var_desc env_vars[] = {
+ /* Array is indexed using enum env_var above.. */
+ { ENV_FILENAME, strlen(ENV_FILENAME) },
+ { ENV_VIRUSNAME, strlen(ENV_VIRUSNAME) }
+};
+
+static char *put_env_var(int var, const char *value)
{
- char *buffer, *pt, *cmd;
- struct cfgstruct *cpt;
+ const struct env_var_desc *desc;
+ int len;
+ char *buf;
+
+ if (var < 0 || var >= N_ENV_VARS) return NULL;
+ if (!value) value = "";
+ desc = &env_vars[var];
+ len = desc->namelen + strlen(value) + sizeof(char)*2;
+ buf = (char *) mcalloc(len, sizeof(char));
+ snprintf(buf, len, "%s=%s", desc->name, value);
+ putenv(buf);
+ return buf;
+}
+static void free_env_var(char **buf)
+{
+ char *eq;
+
+ if (!buf || !*buf) return;
+ if ( (eq = strchr(*buf, '=')) ) {
+ *eq = '\0';
+ putenv(*buf); /* clear old var.. as per SUSv2 */
+ }
+ free(*buf);
+ *buf = 0;
+}
+
+void virusaction(const char *filename, const char *virname, const struct
cfgstruct *copt)
+{
+ pid_t pid;
+ struct cfgstruct *cpt;
if(!(cpt = cfgopt(copt, "VirusEvent")))
return;
- cmd = strdup(cpt->strarg);
+ /* NB: we need to fork here since this function modifies the environment.
+ (Modifications to the env. are not reentrant, but we need to be.) */
+ pid = fork();
+
+ if ( pid == 0 ) {
+ /* child... */
+ char *buffer, *pt, *cmd, *env_filename, *env_virname;
+
+ cmd = strdup(cpt->strarg);
+
+ if((pt = strstr(cmd, "%v"))) {
+ buffer = (char *) mcalloc(strlen(cmd) + strlen(virname) + 10,
sizeof(char));
+ *pt = 0; pt += 2;
+ strcpy(buffer, cmd);
+ strcat(buffer, virname);
+ strcat(buffer, pt);
+ free(cmd);
+ cmd = strdup(buffer);
+ free(buffer);
+ }
+
- if((pt = strstr(cmd, "%v"))) {
- buffer = (char *) mcalloc(strlen(cmd) + strlen(virname) + 10,
sizeof(char));
- *pt = 0; pt += 2;
- strcpy(buffer, cmd);
- strcat(buffer, virname);
- strcat(buffer, pt);
+ env_filename = put_env_var(FILENAME, filename);
+ env_virname = put_env_var(VIRUSNAME, virname);
+
+ /* WARNING: this is uninterruptable ! */
+ exit(system(cmd));
+
+ /* The below is not reached but is here for completeness to remind
+ maintainers that these 3 buffers are still allocated.. */
free(cmd);
- cmd = strdup(buffer);
- free(buffer);
+ free_env_var(&env_filename);
+ free_env_var(&env_virname);
+ } else if (pid > 0) {
+ /* parent */
+ waitpid(pid, NULL, 0);
+ } else {
+ /* error.. */
+ logg("!VirusAction: fork failed.\n");
}
-
- /* WARNING: this is uninterruptable ! */
- system(cmd);
-
- free(cmd);
}
int poll_fd(int fd, int timeout_sec)
diff -ur clamav-devel/clamd/others.h clamav-devel-modified/clamd/others.h
--- clamav-devel/clamd/others.h 2005-03-09 12:32:41.000000000 -0500
+++ clamav-devel-modified/clamd/others.h 2005-03-14 10:08:30.000000000
-0500
@@ -28,7 +28,7 @@
int poll_fd(int fd, int timeout_sec);
int is_fd_connected(int fd);
-void virusaction(const char *virname, const struct cfgstruct *copt);
+void virusaction(const char *filename, const char *virname, const struct
cfgstruct *copt);
int writen(int fd, void *buff, unsigned int count);
#if defined(HAVE_RECVMSG) && (defined(HAVE_ACCRIGHTS_IN_MSGHDR) ||
defined(HAVE_CONTROL_IN_MSGHDR)) && !defined(C_CYGWIN) && !defined(C_OS2)
diff -ur clamav-devel/clamd/scanner.c clamav-devel-modified/clamd/scanner.c
--- clamav-devel/clamd/scanner.c 2005-03-09 12:32:41.000000000 -0500
+++ clamav-devel-modified/clamd/scanner.c 2005-03-14 10:08:30.000000000
-0500
@@ -159,7 +159,7 @@
mdprintf(odesc, "%s: %s FOUND\n", fname,
*virname);
logg("%s: %s FOUND\n", fname, *virname);
- virusaction(*virname, copt);
+ virusaction(fname, *virname, copt);
if(!contscan) {
closedir(dd);
free(fname);
@@ -237,7 +237,7 @@
if(ret == CL_VIRUS) {
mdprintf(odesc, "%s: %s FOUND\n", filename, virname);
logg("%s: %s FOUND\n", filename, virname);
- virusaction(virname, copt);
+ virusaction(filename, virname, copt);
} else if(ret != CL_CLEAN) {
mdprintf(odesc, "%s: %s ERROR\n", filename, cl_strerror(ret));
logg("%s: %s ERROR\n", filename, cl_strerror(ret));
@@ -266,6 +266,7 @@
int ret;
const char *virname;
struct stat statbuf;
+ char fdstr[32];
if(fstat(fd, &statbuf) == -1)
@@ -274,19 +275,21 @@
if(!S_ISREG(statbuf.st_mode))
return -1;
+ snprintf(fdstr, sizeof(fdstr), "fd[%d]", fd);
+
ret = cl_scandesc(fd, &virname, scanned, root, limits, options);
if(ret == CL_VIRUS) {
- mdprintf(odesc, "fd[%d]: %s FOUND\n", fd, virname);
- logg("fd[%d]: %s FOUND\n", fd, virname);
- virusaction(virname, copt);
+ mdprintf(odesc, "%s: %s FOUND\n", fdstr, virname);
+ logg("%s: %s FOUND\n", fdstr, virname);
+ virusaction(fdstr, virname, copt);
} else if(ret != CL_CLEAN) {
- mdprintf(odesc, "fd[%d]: %s ERROR\n", fd, cl_strerror(ret));
- logg("fd[%d]: %s ERROR\n", fd, cl_strerror(ret));
+ mdprintf(odesc, "%s: %s ERROR\n", fdstr, cl_strerror(ret));
+ logg("%s: %s ERROR\n", fdstr, cl_strerror(ret));
} else {
- mdprintf(odesc, "fd[%d]: OK\n", fd);
+ mdprintf(odesc, "%s: OK\n", fdstr);
if(logok)
- logg("fd[%d]: OK\n", fd);
+ logg("%s: OK\n", fdstr);
}
return ret;
@@ -467,7 +470,7 @@
if(ret == CL_VIRUS) {
mdprintf(odesc, "stream: %s FOUND\n", virname);
logg("stream: %s FOUND\n", virname);
- virusaction(virname, copt);
+ virusaction("stream", virname, copt);
} else if(ret != CL_CLEAN) {
mdprintf(odesc, "stream: %s ERROR\n", cl_strerror(ret));
logg("stream: %s ERROR\n", cl_strerror(ret));
_______________________________________________ http://lurker.clamav.net/list/clamav-devel.html
