Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: ... > This issue came up recently again, see: > https://bugzilla.redhat.com/show_bug.cgi?id=447410 > > It appears that the patch that was merged into coreutils ends up calling > matchpathcon_init_prefix() for each file being installed rather than > once upon startup, and without calling matchpathcon_fini() to free the > memory allocated by each matchpathcon_init_prefix() call. > > That makes it slower than necessary and leaks memory. > > See the bug report for the discussion. > > Can we get this corrected in the upstream coreutils? Thanks. Thanks for letting me know. The patch below should do the job. I didn't bother calling matchpathcon_fini, since its 6MB buffer is still reachable. For now, I'm leaving that code ifdef'd out, because the performance penalty is still too high even on rawhide, when performing a few hundred to a thousand separate install commands (about a 20x hit, when installing to /usr). For reference, I did this: # matchpathcon code all ifdef'd out: $ touch k; time ( for i in $(seq 200); do; install k /usr/tmp/k ;done ) 0.08s user 0.30s system 97% cpu 0.391 total # matchpathcon code in use: $ touch k; time ( for i in $(seq 200); do; ./ginstall k /usr/tmp/k ;done ) 7.19s user 1.62s system 99% cpu 8.840 total >From a089634c855312a28f2ff3c2e7c08df5d030e2f5 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@xxxxxxxxxx> Date: Tue, 20 May 2008 17:58:42 +0200 Subject: [PATCH] install: avoid a leak in currently-ifdef'd-out code * src/install.c (setdefaultfilecon) [ENABLE_WHEN_MATCHPATHCON_IS_MORE_EFFICIENT]: Call matchpathcon_init_prefix only once. Suggestion from Stephen Smalley. Reported by Ben Webb in <http://bugzilla.redhat.com/447410>. --- src/install.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/install.c b/src/install.c index 964ab36..b531f45 100644 --- a/src/install.c +++ b/src/install.c @@ -208,6 +208,8 @@ setdefaultfilecon (char const *file) { struct stat st; security_context_t scontext = NULL; + static bool first_call = true; + if (selinux_enabled != 1) { /* Indicate no context found. */ @@ -216,7 +218,7 @@ setdefaultfilecon (char const *file) if (lstat (file, &st) != 0) return; - if (IS_ABSOLUTE_FILE_NAME (file)) + if (first_call && IS_ABSOLUTE_FILE_NAME (file)) { /* Calling matchpathcon_init_prefix (NULL, "/first_component/") is an optimization to minimize the expense of the following @@ -247,6 +249,7 @@ setdefaultfilecon (char const *file) } } } + first_call = false; /* If there's an error determining the context, or it has none, return to allow default context */ -- 1.5.5.1.249.g68ef3 -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.
