diff options
author | Felix Fietkau <nbd@openwrt.org> | 2009-12-09 18:50:49 +0000 |
---|---|---|
committer | Felix Fietkau <nbd@openwrt.org> | 2009-12-09 18:50:49 +0000 |
commit | 679cab88fe71ce59c158f2a6ba99b669a9069f63 (patch) | |
tree | acc30c3875e38895d7b46a93b2d2977b70d3425f /package | |
parent | d5d8a225f886ebe776a75d5a877a5af96f658abf (diff) | |
download | upstream-679cab88fe71ce59c158f2a6ba99b669a9069f63.tar.gz upstream-679cab88fe71ce59c158f2a6ba99b669a9069f63.tar.bz2 upstream-679cab88fe71ce59c158f2a6ba99b669a9069f63.zip |
Fix a memory leak in hotplug2 environment handling. Bump hotplug2 to the latest svn revision, remove obsolete patches.
Memory leak is caused by the way hotplug2 handles environment variables,
using setenv() and unsetenv(). setenv() creates copies of the supplied
strings, but, due to a POSIX blunder, these copies are never destroyed
by unsetenv(), neither in glibc nor uclibc - not until the program
terminates.
Since some events are handled directly in the main process, even when
configured with the "fork" worker, hotplug2 memory usage will keep
growing over time. This can be observed by running "udevtrigger" and
noting the increase in hotplug2 VmRSS after each run.
This patch uses putenv() instead, which leaves storage management to
the caller, so that we can explicitly delete stuff when it's no longer
needed.
Signed-off-by: Aleksandar Radovanovic <biblbroks@sezampro.rs>
SVN-Revision: 18725
Diffstat (limited to 'package')
-rw-r--r-- | package/hotplug2/Makefile | 2 | ||||
-rw-r--r-- | package/hotplug2/patches/100-env_memleak.patch | 64 | ||||
-rw-r--r-- | package/hotplug2/patches/100-recv_check.patch | 12 | ||||
-rw-r--r-- | package/hotplug2/patches/110-static_worker.patch | 174 |
4 files changed, 81 insertions, 171 deletions
diff --git a/package/hotplug2/Makefile b/package/hotplug2/Makefile index b51e5a97f6..2f8fc1477e 100644 --- a/package/hotplug2/Makefile +++ b/package/hotplug2/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=hotplug2 -PKG_REV:=199 +PKG_REV:=201 PKG_VERSION:=$(PKG_REV) PKG_RELEASE:=1 diff --git a/package/hotplug2/patches/100-env_memleak.patch b/package/hotplug2/patches/100-env_memleak.patch new file mode 100644 index 0000000000..31f404029e --- /dev/null +++ b/package/hotplug2/patches/100-env_memleak.patch @@ -0,0 +1,64 @@ +diff -Naur a/action.c b/action.c +--- a/action.c 2009-11-18 13:15:21.000000000 +0000 ++++ b/action.c 2009-11-18 13:11:19.000000000 +0000 +@@ -31,6 +31,30 @@ + } + + /** ++ * Creates a "key=value" string from the given key and value ++ * ++ * @1 Key ++ * @2 Value ++ * ++ * Returns: Newly allocated string in "key=value" form ++ * ++ */ ++static char* alloc_env(const char *key, const char *value) { ++ size_t keylen, vallen; ++ char *combined; ++ ++ keylen = strlen(key); ++ vallen = strlen(value) + 1; ++ ++ combined = xmalloc(keylen + vallen + 1); ++ memcpy(combined, key, keylen); ++ combined[keylen] = '='; ++ memcpy(&combined[keylen + 1], value, vallen); ++ ++ return combined; ++} ++ ++/** + * Choose what action should be taken according to passed settings. + * + * @1 Hotplug settings +@@ -41,16 +65,25 @@ + */ + void action_perform(struct settings_t *settings, struct uevent_t *event) { + int i; ++ char **env; ++ ++ env = xmalloc(sizeof(char *) * event->env_vars_c); ++ ++ for (i = 0; i < event->env_vars_c; i++) { ++ env[i] = alloc_env(event->env_vars[i].key, event->env_vars[i].value); ++ putenv(env[i]); ++ } + +- for (i = 0; i < event->env_vars_c; i++) +- setenv(event->env_vars[i].key, event->env_vars[i].value, 1); +- + if (settings->dumb == 0) { + ruleset_execute(&settings->rules, event, settings); + } else { + action_dumb(settings, event); + } + +- for (i = 0; i < event->env_vars_c; i++) ++ for (i = 0; i < event->env_vars_c; i++) { + unsetenv(event->env_vars[i].key); ++ free(env[i]); ++ } ++ ++ free(env); + } diff --git a/package/hotplug2/patches/100-recv_check.patch b/package/hotplug2/patches/100-recv_check.patch deleted file mode 100644 index 168df47bb0..0000000000 --- a/package/hotplug2/patches/100-recv_check.patch +++ /dev/null @@ -1,12 +0,0 @@ ---- a/hotplug2.c -+++ b/hotplug2.c -@@ -300,6 +300,9 @@ int main(int argc, char *argv[]) { - worker_ctx = settings->worker->module->init(settings); - while (process) { - size = recv(settings->netlink_socket, &buffer, sizeof(buffer), 0); -+ if (size < 0) -+ continue; -+ - uevent = uevent_deserialize(buffer, size); - - if (uevent == NULL) diff --git a/package/hotplug2/patches/110-static_worker.patch b/package/hotplug2/patches/110-static_worker.patch index 453a52d542..bca4aace7c 100644 --- a/package/hotplug2/patches/110-static_worker.patch +++ b/package/hotplug2/patches/110-static_worker.patch @@ -1,164 +1,22 @@ ---- a/Makefile -+++ b/Makefile -@@ -4,7 +4,24 @@ SOFTWARE=hotplug2 - VERSION=1.0-alpha - - BINS=hotplug2 hotplug2-modwrap --SUBDIRS=parser rules workers -+SUBDIRS=parser rules -+ -+hotplug2-objs := \ -+ hotplug2.o netlink.o seqnum.o settings.o uevent.o xmemutils.o \ -+ workers/loader.o parser/parser.o parser/buffer.o parser/token.o \ -+ parser/token_queue.o parser/lexer.o rules/ruleset.o rules/rule.o \ -+ rules/condition.o rules/expression.o rules/execution.o \ -+ rules/command.o -+ -+ifdef STATIC_WORKER -+ ifeq ($(wildcard workers/worker_$(STATIC_WORKER).c),) -+ $(error Worker source worker/worker_$(STATIC_WORKER).c not found) -+ endif -+ hotplug2-objs += action.o workers/worker_$(STATIC_WORKER).o -+else -+ SUBDIRS += workers -+endif -+ - DESTDIR= - - -@@ -13,13 +30,16 @@ all: $(BINS) - install: - $(INSTALL_BIN) $(BINS) $(DESTDIR)/sbin/ - -- --hotplug2: hotplug2.o netlink.o seqnum.o settings.o uevent.o xmemutils.o \ -- workers/loader.o parser/parser.o parser/buffer.o parser/token.o \ -- parser/token_queue.o parser/lexer.o rules/ruleset.o rules/rule.o \ -- rules/condition.o rules/expression.o rules/execution.o \ -- rules/command.o -+hotplug2: $(hotplug2-objs) - - coldplug2: coldplug2.o - - include common.mak -+ -+ifdef STATIC_WORKER -+ CFLAGS += -DSTATIC_WORKER=1 -+else -+ CFLAGS += $(FPIC) -+ LDFLAGS += -ldl -+endif -+ ---- a/common.mak -+++ b/common.mak -@@ -1,7 +1,10 @@ +diff -Naur a/common.mak b/common.mak +--- a/common.mak 2009-11-18 13:15:21.000000000 +0000 ++++ b/common.mak 2009-11-18 13:25:18.000000000 +0000 +@@ -1,7 +1,7 @@ # vim:set sw=8 nosta: --CFLAGS=-Os -Wall -g -fPIC + COPTS=-Os -Wall -g -LDFLAGS=-g -ldl -+COPTS=-Os -Wall -g +LDFLAGS=-g -+ -+CFLAGS=$(COPTS) -+FPIC=-fPIC - INSTALL=install -c -m 644 - INSTALL_BIN=install -c -m 755 ---- a/workers/loader.c -+++ b/workers/loader.c -@@ -1,5 +1,23 @@ - #include "loader.h" - -+#ifdef STATIC_WORKER -+ -+extern struct worker_module_t worker_module; -+static struct loader_ctx_t static_ctx = { -+ .module = &worker_module -+}; -+ -+struct loader_ctx_t *worker_load(const char *name) -+{ -+ return &static_ctx; -+} -+ -+void worker_free(struct loader_ctx_t *ctx) -+{ -+} -+ -+#else -+ - struct loader_ctx_t *worker_load(const char *name) { - struct loader_ctx_t *ctx; - -@@ -12,7 +30,7 @@ struct loader_ctx_t *worker_load(const c - return NULL; - } - -- ctx->module = dlsym(ctx->dl_handle, "module"); -+ ctx->module = dlsym(ctx->dl_handle, "worker_module"); - if (ctx->module == NULL) { - fprintf(stderr, "Loader error: %s\n", dlerror()); - worker_free(ctx); -@@ -31,3 +49,5 @@ void worker_free(struct loader_ctx_t *ct - - free(ctx); - } -+ -+#endif ---- a/hotplug2.c -+++ b/hotplug2.c -@@ -261,17 +261,21 @@ int main(int argc, char *argv[]) { - } - - /* Load the worker. */ -+#ifndef STATIC_WORKER - if (settings->worker_name == NULL) { - fprintf(stderr, "Missing worker name.\n"); - settings_clear(settings); - exit(1); - } -+#endif - settings->worker = worker_load(settings->worker_name); -+#ifndef STATIC_WORKER - if (settings->worker == NULL) { - fprintf(stderr, "Unable to load worker: %s\n", settings->worker_name); - settings_clear(settings); - exit(1); - } -+#endif - - /* Prepare a netlink connection to the kernel. */ - settings->netlink_socket = netlink_init(); ---- a/workers/worker_example.c -+++ b/workers/worker_example.c -@@ -62,7 +62,7 @@ static int worker_example_process(void * - return 0; - } - --struct worker_module_t module = { -+struct worker_module_t worker_module = { - "Hotplug2 example module", - worker_example_init, - worker_example_deinit, ---- a/workers/worker_fork.c -+++ b/workers/worker_fork.c -@@ -443,7 +443,7 @@ static int worker_fork_process(void *in_ - return 0; - } - --struct worker_module_t module = { -+struct worker_module_t worker_module = { - "Hotplug2 forking module", - worker_fork_init, - worker_fork_deinit, ---- a/workers/worker_single.c -+++ b/workers/worker_single.c -@@ -18,7 +18,7 @@ static int worker_single_process(void *s - return 0; - } + CFLAGS=$(COPTS) + FPIC=-fPIC +diff -Naur a/Makefile b/Makefile +--- a/Makefile 2009-11-18 13:15:21.000000000 +0000 ++++ b/Makefile 2009-11-18 13:25:18.000000000 +0000 +@@ -40,5 +40,6 @@ + CFLAGS += -DSTATIC_WORKER=1 + else + CFLAGS += $(FPIC) ++ LDFLAGS += -ldl + endif --struct worker_module_t module = { -+struct worker_module_t worker_module = { - "Hotplug2 single process module", - worker_single_init, - worker_single_deinit, |