[Clamav-devel] clamd: Keep scanning while reloading database
Steven Morgan
smorgan at sourcefire.com
Mon Apr 28 11:58:15 EDT 2014
Thanks for sending a patch, we will look at integrating it with ClamAV.
I have opened bug # 10979 at bugzilla.clamav.net for tracking.
On Mon, Apr 28, 2014 at 10:29 AM, Julius Plenz <plenz at cis.fu-berlin.de>wrote:
> Hi,
>
> * Julius Plenz <plenz at cis.fu-berlin.de> [2014-04-07 20:43]:
> > > 1) Start reload_db() in a background thread, resume scanning, and call
> > > back once the new engine is in place; then exchange the pointers
> > > from old to new engine and free the old one.
> >
> > FWIW, I have implemented this option, and it seems to work just
> > fine.
>
> We have been running ClamAV on all incoming mail for the past week
> while periodically reloading a fresh definition database, and it works
> very well. Any chance of getting this included into upstream?
>
> Current patch against Git tag clamav-0.98.1 pasted below.
>
> Cheers,
>
> Julius
> --
> clamd/server-th.c | 145
> ++++++++++++++++++++++++++++++++----------------------
> clamd/server.h | 7 +++
> 2 files changed, 94 insertions(+), 58 deletions(-)
>
> diff --git a/clamd/server-th.c b/clamd/server-th.c
> index 86a3a48..3df5c9c 100644
> --- a/clamd/server-th.c
> +++ b/clamd/server-th.c
> @@ -64,6 +64,8 @@
> int progexit = 0;
> pthread_mutex_t exit_mutex = PTHREAD_MUTEX_INITIALIZER;
> int reload = 0;
> +volatile int reload_in_progress = 0;
> +struct cl_engine *reload_engine = NULL;
> time_t reloaded_time = 0;
> pthread_mutex_t reload_mutex = PTHREAD_MUTEX_INITIALIZER;
> int sighup = 0;
> @@ -158,40 +160,27 @@ void sighandler_th(int sig)
> logg("$Failed to write to syncpipe\n");
> }
>
> -static struct cl_engine *reload_db(struct cl_engine *engine, unsigned int
> dboptions, const struct optstruct *opts, int do_check, int *ret)
> +static void *reload_db(void *argp)
> {
> - const char *dbdir;
> - int retval;
> - unsigned int sigs = 0;
> - struct cl_settings *settings = NULL;
> -
> - *ret = 0;
> - if(do_check) {
> - if(!dbstat.entries) {
> - logg("No stats for Database check - forcing reload\n");
> - return engine;
> - }
> -
> - if(cl_statchkdir(&dbstat) == 1) {
> - logg("SelfCheck: Database modification detected. Forcing
> reload.\n");
> - return engine;
> - } else {
> - logg("SelfCheck: Database status OK.\n");
> - return NULL;
> - }
> - }
> -
> - /* release old structure */
> - if(engine) {
> + const char *dbdir;
> + int retval;
> + unsigned int sigs = 0;
> + struct cl_settings *settings = NULL;
> + struct cl_engine *engine = NULL;
> +
> + struct reload_db_args *args = (struct reload_db_args *)argp;
> + struct cl_engine *oldengine = args->engine;
> + unsigned int dboptions = args->dboptions;
> + const struct optstruct *opts = args->opts;
> +
> + if(oldengine) {
> /* copy current settings */
> - settings = cl_engine_settings_copy(engine);
> + settings = cl_engine_settings_copy(oldengine);
> if(!settings)
> logg("^Can't make a copy of the current engine settings\n");
> -
> - thrmgr_setactiveengine(NULL);
> - cl_engine_free(engine);
> }
>
> +
> dbdir = optget(opts, "DatabaseDirectory")->strarg;
> logg("Reading databases from %s\n", dbdir);
>
> @@ -201,18 +190,12 @@ static struct cl_engine *reload_db(struct cl_engine
> *engine, unsigned int dbopti
> memset(&dbstat, 0, sizeof(struct cl_stat));
> if((retval = cl_statinidir(dbdir, &dbstat))) {
> logg("!cl_statinidir() failed: %s\n", cl_strerror(retval));
> - *ret = 1;
> - if(settings)
> - cl_engine_settings_free(settings);
> - return NULL;
> + goto err;
> }
>
> if(!(engine = cl_engine_new())) {
> logg("!Can't initialize antivirus engine\n");
> - *ret = 1;
> - if(settings)
> - cl_engine_settings_free(settings);
> - return NULL;
> + goto err;
> }
>
> if(settings) {
> @@ -222,25 +205,38 @@ static struct cl_engine *reload_db(struct cl_engine
> *engine, unsigned int dbopti
> logg("^Using default engine settings\n");
> }
> cl_engine_settings_free(settings);
> + settings = NULL;
> }
>
> if((retval = cl_load(dbdir, engine, &sigs, dboptions))) {
> logg("!reload db failed: %s\n", cl_strerror(retval));
> - cl_engine_free(engine);
> - *ret = 1;
> - return NULL;
> + goto err;
> }
>
> if((retval = cl_engine_compile(engine)) != 0) {
> logg("!Database initialization error: can't compile engine: %s\n",
> cl_strerror(retval));
> - cl_engine_free(engine);
> - *ret = 1;
> - return NULL;
> + goto err;
> }
> logg("Database correctly reloaded (%u signatures)\n", sigs);
>
> - thrmgr_setactiveengine(engine);
> - return engine;
> + pthread_mutex_lock(&reload_mutex);
> + time(&reloaded_time);
> + reload_engine = engine;
> + goto end;
> +
> +err:
> + if(settings)
> + cl_engine_settings_free(settings);
> + if(engine)
> + cl_engine_free(engine);
> +
> + pthread_mutex_lock(&reload_mutex);
> +
> +end:
> + reload_in_progress = 0;
> + pthread_mutex_unlock(&reload_mutex);
> +
> + free(args);
> }
>
> /*
> @@ -713,6 +709,7 @@ int recvloop_th(int *socketds, unsigned nsockets,
> struct cl_engine *engine, unsi
> unsigned long long val;
> size_t i, j, rr_last = 0;
> pthread_t accept_th;
> + pthread_t reload_th;
> pthread_mutex_t fds_mutex = PTHREAD_MUTEX_INITIALIZER;
> pthread_mutex_t recvfds_mutex = PTHREAD_MUTEX_INITIALIZER;
> struct acceptdata acceptdata = ACCEPTDATA_INIT(&fds_mutex,
> &recvfds_mutex);
> @@ -1347,10 +1344,19 @@ int recvloop_th(int *socketds, unsigned nsockets,
> struct cl_engine *engine, unsi
> if(selfchk) {
> time(¤t_time);
> if((current_time - start_time) >= (time_t)selfchk) {
> - if(reload_db(engine, dboptions, opts, TRUE, &ret)) {
> + if(!dbstat.entries) {
> + logg("No stats for Database check - forcing reload\n");
> + pthread_mutex_lock(&reload_mutex);
> + reload = 1;
> + pthread_mutex_unlock(&reload_mutex);
> + }
> + else if(cl_statchkdir(&dbstat) == 1) {
> + logg("SelfCheck: Database modification detected.
> Forcing reload.\n");
> pthread_mutex_lock(&reload_mutex);
> reload = 1;
> pthread_mutex_unlock(&reload_mutex);
> + } else {
> + logg("SelfCheck: Database status OK.\n");
> }
> time(&start_time);
> }
> @@ -1358,34 +1364,57 @@ int recvloop_th(int *socketds, unsigned nsockets,
> struct cl_engine *engine, unsi
>
> /* DB reload */
> pthread_mutex_lock(&reload_mutex);
> - if(reload) {
> + if(reload && reload_in_progress) {
> + logg("Database reload already in progress; ignoring reload
> request.\n");
> + reload = 0;
> + pthread_mutex_unlock(&reload_mutex);
> + } else if(reload) {
> pthread_mutex_unlock(&reload_mutex);
> + struct reload_db_args *reload_db_args;
> + reload_db_args = (struct reload_db_args *)
> malloc(sizeof(struct reload_db_args));
> + if (reload_db_args)
> + {
> + reload_db_args->engine = engine;
> + reload_db_args->dboptions = dboptions;
> + reload_db_args->opts = opts;
> +
> + pthread_mutex_lock(&reload_mutex);
> + reload = 0;
> + reload_in_progress = 1;
> +
> + if (pthread_create(&reload_th, NULL, reload_db,
> reload_db_args) != 0) {
> + logg("*Error dispatching DB reload thread!\n");
> + reload_in_progress = 0;
> + free(reload_db_args);
> + }
> + pthread_mutex_unlock(&reload_mutex);
> + }
> + } else if(reload_engine != NULL) {
> #if defined(FANOTIFY) || defined(CLAMAUTH)
> + pthread_mutex_unlock(&reload_mutex);
> if(optget(opts, "ScanOnAccess")->enabled && tharg) {
> logg("Restarting on-access scan\n");
> pthread_kill(fan_pid, SIGUSR1);
> pthread_join(fan_pid, NULL);
> }
> -#endif
> - engine = reload_db(engine, dboptions, opts, FALSE, &ret);
> - if(ret) {
> - logg("Terminating because of a fatal error.\n");
> - if(new_sd >= 0)
> - closesocket(new_sd);
> - break;
> - }
> -
> pthread_mutex_lock(&reload_mutex);
> - reload = 0;
> - time(&reloaded_time);
> +#endif
> + static struct cl_engine *tmp;
> + tmp = engine;
> + engine = reload_engine;
> + thrmgr_setactiveengine(engine);
> + time(&start_time);
> + reload_engine = NULL;
> pthread_mutex_unlock(&reload_mutex);
> +
> + cl_engine_free(tmp);
> +
> #if defined(FANOTIFY) || defined(CLAMAUTH)
> if(optget(opts, "ScanOnAccess")->enabled && tharg) {
> tharg->engine = engine;
> pthread_create(&fan_pid, &fan_attr, fan_th, tharg);
> }
> #endif
> - time(&start_time);
> } else {
> pthread_mutex_unlock(&reload_mutex);
> }
> diff --git a/clamd/server.h b/clamd/server.h
> index bc2a296..e171ea9 100644
> --- a/clamd/server.h
> +++ b/clamd/server.h
> @@ -46,6 +46,13 @@ struct thrwarg {
> unsigned int options;
> };
>
> +/* reload_db arguments */
> +struct reload_db_args {
> + struct cl_engine *engine;
> + unsigned int dboptions;
> + const struct optstruct *opts;
> +};
> +
> int recvloop_th(int *socketds, unsigned nsockets, struct cl_engine
> *engine, unsigned int dboptions, const struct optstruct *opts);
> void sighandler(int sig);
> void sighandler_th(int sig);
> --
> 1.9.0-zedat
>
> _______________________________________________
> http://lurker.clamav.net/list/clamav-devel.html
> Please submit your patches to our Bugzilla: http://bugs.clamav.net
>
More information about the clamav-devel
mailing list