Bug 447410 - 'install' consumes all system memory
Summary: 'install' consumes all system memory
Alias: None
Product: Fedora
Classification: Fedora
Component: coreutils
Version: 9
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Ondrej Vasik
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2008-05-19 20:22 UTC by Ben Webb
Modified: 2008-07-26 06:07 UTC (History)
4 users (show)

Fixed In Version: coreutils-6.10-23.fc9
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2008-05-22 12:26:01 UTC

Attachments (Terms of Use)
Patch to call matchpathcon_init_prefix just once and free memory afterwards (2.58 KB, patch)
2008-05-20 15:04 UTC, Ondrej Vasik
no flags Details | Diff

Description Ben Webb 2008-05-19 20:22:01 UTC
Description of problem:
'install' uses all system memory and is eventually killed, even when installing
0-byte files.

Version-Release number of selected component (if applicable):
coreutils-6.10-22.fc9 (i386 and x86_64)

How reproducible:

Steps to Reproduce:
1. mkdir tmp
2. for num in `seq 1000`; do touch tmp/img$num.png; done
3. /usr/bin/install tmp/*.png /var/tmp/

Actual results:
The 'install' process uses all the memory on the machine - I let it run until it
consumed 470M resident 1.7G swap before killing it on a 32-bit machine. It ran
out of system memory (and was killed by the kernel) on an x86_64 system with 4G
physical memory.

Expected results:
'install' installs all files in /var/tmp/.

Additional info:
Tested on both i386 and x86_64 systems, updated Fedora 9.

Comment 1 Ondrej Vasik 2008-05-19 21:27:52 UTC
Thanks for report, it looks like quiet a big memory leak in install and dozens
of SELinux policies for /var/tmp/ are responsible for that issue.

Comment 2 Ondrej Vasik 2008-05-19 22:01:47 UTC
libselinux matchpathcon_init_prefix function seems to be leaking.

==8250== 247,375,872 (3,889,600 direct, 243,486,272 indirect) bytes in 110
blocks are definitely lost in loss record 68 of 68
==8250==    at 0x4005400: malloc (vg_replace_malloc.c:149)
==8250==    by 0x4D9055: matchpathcon_init_prefix (in /lib/libselinux.so.1)
==8250==    by 0x804A5F6: (within /usr/bin/install)
==8250==    by 0x804B171: (within /usr/bin/install)
==8250==    by 0x234DEB: (below main) (in /lib/libc-2.5.so)

Redirecting component and adding myself to cc( Note: I have noticed that the
memory leak was reported on
, so I guess problem is maybe already solved in rawhide.)

Comment 3 Stephen Smalley 2008-05-20 12:41:33 UTC
What libselinux version was used?
A couple of memory leaks in matchpathcon were fixed in 2.0.62.
Also, does install invoke matchpathcon_init_prefix() more than once without
first invoking matchpathcon_fini() to release the previously allocated memory?
Note btw that we would like to migrate all users of matchpathcon* to using
selabel_open/lookup/close going forward.

Comment 4 Stephen Smalley 2008-05-20 12:56:27 UTC
I looked at the source code, and it appears that it calls
matchpathcon_init_prefix() for each file and never calls matchpathcon_fini().
It shouldn't be calling matchpathcon_init_prefix() on each file, only once at
startup for the entire run with the destination as the prefix IIUC.  Then it
just calls matchpathcon() on each file.  The matchpathcon_init_prefix() call
loads all file contexts entries that match the specified prefix into memory for
later use by matchpathcon().
And then it should call matchpathcon_fini() prior to exiting if you want it to
be clean valgrind-wise at exit time.

Comment 5 Stephen Smalley 2008-05-20 12:57:21 UTC
So to clarify - AFAICS, this is a bug in install, not in libselinux.
Failure to properly use the matchpathcon_init_prefix (call once during startup
with the destination as the prefix, not on each file), and failure to call
matchpathcon_fini on the exit path.

Comment 6 Ondrej Vasik 2008-05-20 13:24:37 UTC
Thanks for clarification, it is comming from the upstream commit
, although upstream disabled that part completely. Will fix the worst in install
by usage of matchpathcon_fini soon.

Comment 7 Eric Paris 2008-05-20 14:12:49 UTC
upstream disabled it completely due to performance problems as I recall.  Fixing
this the right way (only calling init one time) should fix both the leak and
GREATLY reduce the performance impact....

please fix it that way and be sure to push it upstream....

Comment 8 Stephen Smalley 2008-05-20 14:41:44 UTC
I've followed up on that thread on selinux list (btw I find
http://marc.info/?l=selinux to be easier to search and link to than the engarde
archives) to raise this issue to the attention of the coreutils maintainer.

Comment 9 Ondrej Vasik 2008-05-20 15:04:55 UTC
Created attachment 306138 [details]
Patch to call matchpathcon_init_prefix just once and free memory afterwards

Yep, that's the plan, that upstream disabling caused reopening a few bugzillas,
therefore I activated and that resulted into that issue.
Current patch has following results:
Installation of 1000 files to /var/tmp/:
Completely without setting default scontext: 0.1012 s.
With new patch				   : 0.2938 s.

Comment 10 Ondrej Vasik 2008-05-20 15:06:14 UTC
And adding upstream maintainer to cc...

Comment 11 Stephen Smalley 2008-05-20 15:11:21 UTC
I don't believe you want to use file[0] as part of the prefix computation, just
the destination directory - otherwise, can't you potentially end up with a
prefix that is overly specific to the first file in the list?  Maybe not - I
didn't look closely.
Also, for the time results above, could you also try with the old code setting
the default context to see how it much of a difference this patch makes relative
to it?

Comment 12 Ondrej Vasik 2008-05-20 15:33:54 UTC
About the file[0] - I'm not really sure now, but I don't know about other easy
way for just once calling matchpathcon_init_prefix. Maybe Jim will have some way
how to solve it better as usually. 

Just for comparision:
F-7 approach(without matchpathcon_init_prefix "optimalization") : 0.4526 s.
F-9 current situation(matchpatcon_init_prefix for each file, no
matchpathcon_fini() call)                                       : 87.4651 s.

Comment 13 Ondrej Vasik 2008-05-20 15:54:09 UTC
Will check the way of the set_prefix() function just with dest_dir to avoid
possibility of too specific list of contexts. This patch is just temporary
solution of problem with huge memory leak in install.

And for completeness of statistics - pure upstream coreutils-6.11 without Fedora
patches : 0.1534 s.

Comment 14 Fedora Update System 2008-05-20 16:13:17 UTC
coreutils-6.10-23.fc9 has been submitted as an update for Fedora 9

Comment 15 Jim Meyering 2008-05-20 16:23:40 UTC
Wow.  I do see a huge leak when running /usr/bin/install ... /usr/tmp in
rawhide's coreutils-6.10-22.fc9.x86_64 with two or more files. Actually, there
is no leak for a single file.  But with N files, leaked memory comes to about
(N-1)*6MB.  I can reproduce using stock upstream coreutils only if I compile-in
the code that is currently ifdef'd out.

I've just posted a patch to fix the leak (in still-ifdef'd out block) here:

Comment 16 Jim Meyering 2008-05-20 16:26:24 UTC
Stephen, to answer your question in comment #11, the setdefaultfilecon argument
is always the full name (not necessarily absolute) of a *destination* file, and
since all files are being installed into the same top-level directory, the
existing code should be fine.

Comment 17 Fedora Update System 2008-05-21 11:11:26 UTC
coreutils-6.10-23.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 18 Fedora Update System 2008-07-26 06:07:08 UTC
coreutils-6.10-23.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.