locks on unlinked files leak memory in apparmor

[Domas Mituzas]

Apparmor is leaks memory when unlinked files are locked by confined processes.

TEST CASE

Confining the following program:

#include <stdio.h>
#include <unistd.h>

int main(void)
{
        int i;
        int fd = open("/tmp/.lockfile", O_RDWR|O_CREAT);
        unlink("/tmp/.lockfile");

        fork();
        fork();
        fork();
        fork();

        for (i = 0; i < 5000; i++) {
                struct flock lock;
                lock.l_type = F_WRLCK;
                lock.l_start = 0;
                lock.l_whence = SEEK_END;
                lock.l_len = 0;
                fcntl(fd,F_SETLKW, &lock);
                lock.l_type = F_UNLCK;
                fcntl(fd, F_SETLKW, &lock);
        }

        return 0;
}

with an apparmor policy similar to the following (place the policy in /etc/apparmor.d and then do 'sudo /etc/init.d/apparmor restart' to reload policy):

#include <tunables/global>
/PATH/TO/YOUR/COMPILED/BINARY flags=(audit) {
  #include <abstractions/base>
  #include <abstractions/mysql>
  #include <abstractions/nameservice>

  capability kill,
  capability net_bind_service,
  capability setgid,
  capability setuid,

  # Major libs
  /lib/ld-*.so mr,
  /lib/libc-*.so mr,
  /lib/libpthread-*.so mr,
  /lib/librt-*.so mr,

  /tmp/* rwk,

}

(You'll need to change /PATH/TO/YOUR/COMPILED/BINARY in the above profile to point the location of the compiled program.)

While running slabtop in another terminal, run the program.
Without the fix, slabtop should see an increase use of kernel memory, typically the kamlloc-256 slab.
With the fix in place, there shouldn't be much change in slabtop's reported output.

/var/log/messages should get a number of audit events (this confirms that confinement is applied to the binary in question).

===

SRU Justification

Justification: apparmour will leak memory leading to OOM, hangs, or crashes

Impact: certain workloads will cause memory to be leaked for each operation, specifically any name check on removed files

Fix Description: free the memory earlier in the error path

Patch: http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-jaunty.git;a=commit;h=537f85127f05d91f3a464cc3c1808fd8ea75c606

Risks: very well contained and obvious change so this should be low risk. patch is upstream (apparmour)

TEST CASE: see above

[Domas Mituzas (domas-mituzas)]

and, of course:

Linux srv47 2.6.24-19-server #1 SMP Wed Aug 20 18:43:06 UTC 2008 x86_64 GNU/Linux

hardy with all updates

[Kees Cook (kees)]

Thanks for the report! If AppArmor is disabled, does the leak go away? What is the specific configuration items in apache, and the common workload on the machine? I.e. what is the best way to reproduce this problem starting from a stock install of Hardy?

[Domas Mituzas (domas-mituzas)]

testcase:
#include <fcntl.h>
#include <stdio.h>

main() {

int fd = open("/tmp/.lockfile", O_RDWR|O_CREAT);
unlink("/tmp/.lockfile");

fork();
fork();
fork();
fork();

while(1) {
 struct flock lock;
 lock.l_type=F_WRLCK;
 lock.l_start=0;
 lock.l_whence=SEEK_END;
 lock.l_len=0;
 fcntl(fd,F_SETLKW,&lock);
 lock.l_type=F_UNLCK;
 fcntl(fd,F_SETLKW,&lock);
}

}

Profile:
# Last Modified: Fri Dec 5 13:59:51 2008
#include <tunables/global>
/usr/local/sbin/domasaatest flags=(audit) {
  #include <abstractions/base>
  #include <abstractions/mysql>
  #include <abstractions/nameservice>

  capability kill,
  capability net_bind_service,
  capability setgid,
  capability setuid,

  # Major libs
  /lib/ld-*.so mr,
  /lib/libc-*.so mr,
  /lib/libpthread-*.so mr,
  /lib/librt-*.so mr,

  /tmp/* rwk,

}

[Domas Mituzas (domas-mituzas)]

Testcase immediately leaks memory, as well as kernel message buffer gets lots of smashed buffer crap:

http://p.defau.lt/?HxHScO_HJyFcgLVE2Fp8qw

[Domas Mituzas (domas-mituzas)]

One more detail, this is excerpt from /proc/slabinfo:

kmalloc-1024 806 816 1024 4 1 : tunables 0 0 0 : slabdata 204 204 0
kmalloc-512 517 528 512 8 1 : tunables 0 0 0 : slabdata 66 66 0
kmalloc-256 907300 907328 256 16 1 : tunables 0 0 0 : slabdata 56708 56708 0
kmalloc-128 566 704 128 32 1 : tunables 0 0 0 : slabdata 22 22 0
kmalloc-64 28608 28800 64 64 1 : tunables 0 0 0 : slabdata 450 450 0

the kmalloc-256 is the one that doesn't stop growing

[Kees Cook (kees)]

Yup, your testcase killed my hardy vm, but only when AppArmor was confining the testcase.

[Kees Cook (kees)]

This affects intrepid and jaunty as well.

[Steve Beattie (sbeattie)]

On Sat, Feb 14, 2009 at 09:16:37PM -0000, Kees Cook wrote:
> Yup, your testcase killed my hardy vm, but only when AppArmor was
> confining the testcase.

Does it do the same if the audit flag is removed from the profile?

--
Steve Beattie
<email address hidden>
http://NxNW.org/~steve/

[Domas Mituzas (domas-mituzas)]

Steve,

it does same in all modes:

aa-complain
aa-audit
aa-enforce

[Domas Mituzas (domas-mituzas)]

OK, sorry folks for having too difficult start, I ended up with extremely simple testcase, leaking 100MB/s:

#include <fcntl.h>
#include <stdio.h>

main() {
    int fd = open("/tmp/.lockfile", O_RDWR | O_CREAT);
    unlink("/tmp/.lockfile");
    while (1) write(fd,"a",1);
}

I'm changing the issue summary yet again.

[Steve Beattie (sbeattie)]

It looks like the issue is the constructed pathname dealing with the deleted file; commenting out the unlink() call in the testcase causes no memory growth, in any of the modes (you do need to remember to manually delete the lockfile if it exists before each testrun).

(FYI, you can use slabtop to see the growth. I also limited the while loop locally to do 1000 iterations, which makes the leak visible, but doesn't cause the kernel to crash.)

[Domas Mituzas (domas-mituzas)]

How did you make the loop run only 1000 times?!!!? :)
Thanks for 'slabtop' hint - nice tool for tourists from userland.

Now, let me put my MySQL hat on:
- MySQL ships by default with apparmor profile
- MySQL uses unlinked files quite a bit (implicit temptables, sort buffer overflows, etc)
So, this affects mysql-server package a lot too.

[Domas Mituzas (domas-mituzas)]

damn, after all, it may be just fcntl() - I can't reproduce the write() case anymore, and I wonder if I ever could.
teaches me that C is a compiled language!

[Steve Beattie (sbeattie)]

Upstream (jjohansen) thinks that
https://forgesvn1.novell.com/viewsvn/apparmor?view=rev&revision=1380
fixes the issue; I haven't tested it yet.

--
Steve Beattie
<email address hidden>
http://NxNW.org/~steve/

[Steve Beattie (sbeattie)]

I've created test kernels for intrepid in my ppa at https://launchpad.net/~sbeattie/+archive/ppa based on the previously mentioned upstream commit. Testing locally shows that this does address the issue. I'm working on generating kernels for testing on hardy.

You'll want to follow the instructions at https://help.launchpad.net/Packaging/PPA#Adding%20a%20PPA%20to%20your%20Ubuntu%20repositories for installing from this ppa. The kernel this is based off of is the current kernel in intrepid-proposed.

I've attached the git format-patch generated diff.

Note that this kernel is for testing only. Once this issue is verified, the patch will be forwarded into the update process.

[Steve Beattie (sbeattie)]

There are now test kernels for hardy in my ppa at https://launchpad.net/~sbeattie/+archive/ppa based off of the same patch. Again, local testing shows that the rapid memory leak in the apparmor code in the unpatched kernel is addressed by this fix.

Feedback is appreciated. You'll want to follow the instructions at https://help.launchpad.net/Packaging/PPA#Adding%20a%20PPA%20to%20your%20Ubuntu%20repositories for installing from this ppa. The kernel this is based off of is the current kernel in hardy-proposed. Thanks.

[Tim Gardner (timg-tpi)]

http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-jaunty.git;a=commit;h=537f85127f05d91f3a464cc3c1808fd8ea75c606

[Domas Mituzas (domas-mituzas)]

It fixed at least for my VM tests (though they were mostly same as initial testcase).
Thanks Steve.

[Launchpad Janitor (janitor)]

This bug was fixed in the package linux - 2.6.28-8.27

---------------
linux (2.6.28-8.27) jaunty; urgency=low

  [ Amit Kucheria ]

  * Updating configs (arm:ixp4xx)

  [ Andy Whitcroft ]

  * SAUCE: enable Intel HDMI output

  [ Manoj Iyer ]

  * SAUCE: Added quirk for Linksys WUSB600N USB wifi-n networking adapter
    - LP: #323473

  [ Steve Beattie ]

  * fix apparmor memory leak on unlinked file ops
    - LP: #329489

  [ Tim Gardner ]

  * SAUCE: Dell XPS710 reboot quirk
    - LP: #323592
  * SAUCE: (drop after 2.6.28) ieee80211: Add infrastructure to obsolete
    scan results
    - LP: #336055
  * Add modules.order to the linux-image package.

  [ Upstream Kernel Changes ]

  * iwlwifi: fix time interval misuse in iwl_poll_{direct_}bit
  * x86: only scan the root bus in early PCI quirks
    - LP: #267295
  * ALSA: hda - Intel HDMI audio support
  * ALSA: hda - Fix unused function in patch_intelhdmi.c
  * ALSA: handle SiI1392 HDMI codec in patch_intelhdmi.c
  * ALSA: hda-intel: reorder HDMI audio enabling sequence
  * ALSA: introduce snd_print_pcm_rates()
  * ALSA: create hda_eld.c for ELD routines and proc interface
  * ALSA: ELD proc interface for HDMI sinks
  * ALSA: hda: make standalone hdmi_fill_audio_infoframe()
  * ALSA: hda: make global snd_print_channel_allocation()
  * ALSA: hda: HDMI channel allocations for audio infoframe
  * ALSA: hda: HDMI channel mapping cleanups
  * ALSA: hda: minor code cleanups
  * ALSA: hda: rename sink_eld to hdmi_eld
  * ALSA: hda - Release ELD proc file
  * ALSA: hda - minor HDMI code cleanups
  * ALSA: hda - report selected CA index for Audio InfoFrame
  * ALSA: hda - Add Intel vendor id string

 -- Tim Gardner <email address hidden> Wed, 25 Feb 2009 14:23:46 -0700

[Steve Beattie (sbeattie)]

This bug is in the kernel portion of apparmor, closing the userspace apparmor component tasks. Sorry for the noise.

[Martin Pitt (pitti)]

Accepted intrepid into linux-proposed; please test and give feedback here. Please see https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Martin Pitt (pitti)]

Accepted linux into hardy-proposed; please test and give feedback here. Please see https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Launchpad Janitor (janitor)]

This bug was fixed in the package linux - 2.6.27-11.31

---------------
linux (2.6.27-11.31) intrepid-security; urgency=low

  [ Steve Beattie ]

  * fix apparmor memory leak on deleted file ops Bug: #329489
    - LP: #329489

  [ Upstream Kernel Changes ]

  * sctp: Avoid memory overflow while FWD-TSN chunk is received with bad
    stream ID
    - CVE-2009-0065
  * prevent kprobes from catching spurious page faults
    - CVE-2009-0605
  * net: 4 bytes kernel memory disclosure in SO_BSDCOMPAT gsopt try #2
    - CVE-2009-0676
  * copy_process: fix CLONE_PARENT && parent_exec_id interaction
    - CVE-2009-0028
  * security: introduce missing kfree
    - CVE-2009-0031
  * eCryptfs: check readlink result was not an error before using it
    - CVE-2009-0269
  * dell_rbu: use scnprintf() instead of less secure sprintf()
    - CVE-2009-0322
  * drivers/net/skfp: if !capable(CAP_NET_ADMIN): inverted logic
    - CVE-2009-0675
  * ext4: Initialize the new group descriptor when resizing the filesystem
    - CVE-2009-0745
  * ext4: Add sanity check to make_indexed_dir
    - CVE-2009-0746
  * ext4: only use i_size_high for regular files
    - CVE-2009-0747
  * ext4: Add sanity checks for the superblock before mounting the
    filesystem
    - CVE-2009-0748
  * x86-64: syscall-audit: fix 32/64 syscall hole
    - CVE-2009-0834
  * x86-64: seccomp: fix 32/64 syscall hole
    - CVE-2009-0835
  * shm: fix shmctl(SHM_INFO) lockup with !CONFIG_SHMEM
    - CVE-2009-0859
  * udf:SAUCE (drop after 2.6.30): Fix oops when invalid character in
    filename occurs
    - LP: #321606
  * Add '-fwrapv' to gcc CFLAGS
    - LP: #348015
  * Move cc-option to below arch-specific setup
    - LP: #348015
  * Fix memory corruption in console selection
    - CVE-2009-1046

 -- Stefan Bader <email address hidden> Mon, 16 Mar 2009 18:48:27 +0100

[Launchpad Janitor (janitor)]

This bug was fixed in the package linux - 2.6.24-23.52

---------------
linux (2.6.24-23.52) hardy-security; urgency=low

  [Stefan Bader]

  * rt: Fix FTBS caused by shm changes
    - CVE-2009-0859

  [Steve Beattie]

  * fix apparmor memory leak on deleted file ops Bug: #329489
    - LP: #329489

  [Upstream Kernel Changes]

  * NFS: Remove the buggy lock-if-signalled case from do_setlk()
    - CVE-2008-4307
  * sctp: Avoid memory overflow while FWD-TSN chunk is received with bad
    stream ID
    - CVE-2009-0065
  * net: 4 bytes kernel memory disclosure in SO_BSDCOMPAT gsopt try #2
    - CVE-2009-0676
  * sparc: Fix mremap address range validation.
    - CVE-2008-6107
  * copy_process: fix CLONE_PARENT && parent_exec_id interaction
    - CVE-2009-0028
  * security: introduce missing kfree
    - CVE-2009-0031
  * eCryptfs: check readlink result was not an error before using it
    - CVE-2009-0269
  * dell_rbu: use scnprintf() instead of less secure sprintf()
    - CVE-2009-0322
  * drivers/net/skfp: if !capable(CAP_NET_ADMIN): inverted logic
    - CVE-2009-0675
  * Ext4: Fix online resize block group descriptor corruption
    - CVE-2009-0745
  * ext4: Initialize the new group descriptor when resizing the filesystem
    - CVE-2009-0745
  * ext4: Add sanity check to make_indexed_dir
    - CVE-2009-0746
  * x86-64: syscall-audit: fix 32/64 syscall hole
    - CVE-2009-0834
  * x86-64: seccomp: fix 32/64 syscall hole
    - CVE-2009-0835
  * shm: fix shmctl(SHM_INFO) lockup with !CONFIG_SHMEM
    - CVE-2009-0859
  * apparmor: Fix handling of larger number of profiles
    - LP: #345144
  * udf:SAUCE (drop after 2.6.30): Fix oops when invalid character in
    filename occurs
    - LP: #321606
  * Fix memory corruption in console selection
    - CVE-2009-1046
  * SPARC64: Loosen checks in exception table handling.
    - LP: #301608, #349655

 -- Stefan Bader <email address hidden> Mon, 16 Mar 2009 18:39:14 +0100