Bug 447410 - 'install' consumes all system memory
'install' consumes all system memory
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: coreutils (Show other bugs)
9
All Linux
low Severity high
: ---
: ---
Assigned To: Ondrej Vasik
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-19 16:22 EDT by Ben Webb
Modified: 2008-07-26 02:07 EDT (History)
4 users (show)

See Also:
Fixed In Version: coreutils-6.10-23.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-22 08:26:01 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


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

  None (edit)
Description Ben Webb 2008-05-19 16:22:01 EDT
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.
Comment 1 Ondrej Vasik 2008-05-19 17:27:52 EDT
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 18:01:47 EDT
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.)
Comment 3 Stephen Smalley 2008-05-20 08:41:33 EDT
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 08:56:27 EDT
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 08:57:21 EDT
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 09:24:37 EDT
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.
Comment 7 Eric Paris 2008-05-20 10:12:49 EDT
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 10:41:44 EDT
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 11:04:55 EDT
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 11:06:14 EDT
And adding upstream maintainer to cc...
Comment 11 Stephen Smalley 2008-05-20 11:11:21 EDT
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 11:33:54 EDT
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 11:54:09 EDT
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 12:13:17 EDT
coreutils-6.10-23.fc9 has been submitted as an update for Fedora 9
Comment 15 Jim Meyering 2008-05-20 12:23:40 EDT
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
Comment 16 Jim Meyering 2008-05-20 12:26:24 EDT
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 07:11:26 EDT
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 02:07:08 EDT
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.