locks on unlinked files leak memory in apparmor

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

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?

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,

}

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

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

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

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

This affects intrepid and jaunty as well.

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/

Steve,

it does same in all modes:

aa-complain
aa-audit
aa-enforce

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.

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.)

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.

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!

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/

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.

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.

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

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

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

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

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!

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!

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

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