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: Always 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.
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.
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 http://www.engardelinux.org/modules/index/list_archives.cgi?list=selinux&page=1141.html&month=2008- , so I guess problem is maybe already solved in rawhide.)
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.
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.
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.
Thanks for clarification, it is comming from the upstream commit http://www.engardelinux.org/modules/index/list_archives.cgi?list=selinux&page=0112.html&month=2007-12 , although upstream disabled that part completely. Will fix the worst in install by usage of matchpathcon_fini soon.
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....
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.
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.
And adding upstream maintainer to cc...
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?
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.
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.
coreutils-6.10-23.fc9 has been submitted as an update for Fedora 9
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: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/11815/focus=13599
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.
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.