the hardware hook blows away /proc/$pid/cmdline details

Bug #657091 reported by Kees Cook
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
apparmor (Ubuntu)
Fix Released
Low
Jamie Strandboge
Lucid
Fix Released
Low
Jamie Strandboge
Maverick
Fix Released
Low
Jamie Strandboge
Natty
Fix Released
Low
Jamie Strandboge
apport (Ubuntu)
Fix Released
Undecided
Unassigned
Lucid
Won't Fix
Undecided
Unassigned
Maverick
Won't Fix
Undecided
Unassigned
Natty
Fix Released
Undecided
Unassigned

Bug Description

SRU Justification

1. impact of the bug is minimal for stable releases, since apport is disabled by default. However, being able to run 'ubuntu-bug', 'apport-bug' or 'apport-collect' is useful for developers trying to get more information from users.

2. This has been addressed in the development branch

3. Patch is very small. See comment #3.

4. TEST CASE:
$ sudo apport-bug apparmor --save /tmp/apport.txt

Examine /tmp/apport.txt. It should have something like:
ProcKernelCmdline: root=UUID=378db089-c0de-431c-b57d-9675fa5e6319 ro quiet splash

5. The regression potential of the patch is very low, as it updates the apport hook which is not enabled by default in the stable releases.

Initial report

21:34 < kees> self['ProcCmdline'] = _read_file('/proc/' + pid + '/cmdline').rstrip('\0')
21:34 < kees> vs
21:34 < kees> ./apport/hookutils.py: attach_file(report, '/proc/cmdline', 'ProcCmdLine')

The hardware hook is destroying the /proc/$pid/cmdline information.

ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: apport 1.14.1-0ubuntu8
ProcVersionSignature: Ubuntu 2.6.35-22.33-generic 2.6.35.4
Uname: Linux 2.6.35-22-generic x86_64
ApportLog:

Architecture: amd64
CrashReports:
 600:0:0:2142829:2010-10-02 08:42:35.061202818 -0700:2010-10-02 08:42:35.041202736 -0700:/var/crash/libpam-ck-connector.0.crash
 600:0:0:2142491:2010-10-02 08:42:35.021202650 -0700:2010-10-02 08:42:34.691201277 -0700:/var/crash/grub-pc.0.crash
 600:0:0:2143151:2010-10-02 08:42:35.291203777 -0700:2010-10-02 08:42:35.261203650 -0700:/var/crash/libpam-gnome-keyring.0.crash
Date: Fri Oct 8 14:35:30 2010
PackageArchitecture: all
ProcEnviron:
 LANGUAGE=en_US:en
 PATH=(custom, user)
 LANG=en_US.utf8
 SHELL=/bin/bash
SourcePackage: apport

Revision history for this message
Kees Cook (kees) wrote :
Changed in apport (Ubuntu):
status: New → Fix Committed
Revision history for this message
Kees Cook (kees) wrote :

Fixed in bzr commit 1791.

=== modified file 'apport/hookutils.py'
--- apport/hookutils.py 2010-06-23 08:05:56 +0000
+++ apport/hookutils.py 2010-10-08 21:36:25 +0000
@@ -109,7 +109,7 @@

     attach_file(report, '/proc/interrupts', 'ProcInterrupts')
     attach_file(report, '/proc/cpuinfo', 'ProcCpuinfo')
- attach_file(report, '/proc/cmdline', 'ProcCmdLine')
+ attach_file(report, '/proc/cmdline', 'ProcKernelCmdLine')
     attach_file(report, '/proc/modules', 'ProcModules')
     attach_file(report, '/var/log/udev', 'UdevLog')

Revision history for this message
Steve Beattie (sbeattie) wrote :

The apparmor apport hook does not invoke the attach_hardware hook, but also attaches /proc/cmdline directly in the same way, with the same clobbering effect. It needs a similar fix like so:

=== modified file 'debian/apport/source_apparmor.py'
--- debian/apport/source_apparmor.py 2009-11-11 20:21:21 +0000
+++ debian/apport/source_apparmor.py 2010-10-09 02:11:01 +0000
@@ -27,7 +27,7 @@

 def add_info(report):
     attach_file(report, '/proc/version_signature', 'ProcVersionSignature')
- attach_file(report, '/proc/cmdline', 'ProcCmdline')
+ attach_file(report, '/proc/cmdline', 'ProcKernelCmdline')

     sec_re = re.compile('audit\(|apparmor|selinux|security', re.IGNORECASE)
     report['KernLog'] = recent_kernlog(sec_re)

Changed in apparmor (Ubuntu):
status: New → Triaged
importance: Undecided → Low
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

SRU

1. impact of the bug is minimal for stable releases, since apport is disabled by default. However, being able to run 'ubuntu-bug', 'apport-bug' or 'apport-collect' is useful for developers trying to get more information from users.

2. This has not been addressed in the development branch (it isn't open yet).

3. Patch is very small. See comment #3.

4. TEST CASE:
$ sudo apport-bug -p apparmor --save /tmp/apport.txt

Examine /tmp/apport.txt. It should have something like:
ProcKernelCmdline: root=UUID=378db089-c0de-431c-b57d-9675fa5e6319 ro quiet splash

5. The regression potential of the patch is very low, as it updates the apport hook which is not enabled by default in the stable releases.

Changed in apport (Ubuntu Maverick):
status: New → Fix Committed
Changed in apport (Ubuntu Natty):
status: Fix Committed → Triaged
Changed in apparmor (Ubuntu Maverick):
status: New → In Progress
importance: Undecided → Low
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in apparmor (Ubuntu Natty):
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in apparmor (Ubuntu Maverick):
milestone: none → maverick-updates
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Uploaded 2.5.1-0ubuntu0.10.10.1 to maverick-proposed.

Changed in apparmor (Ubuntu Maverick):
milestone: maverick-updates → none
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apparmor - 2.5.1-0ubuntu1

---------------
apparmor (2.5.1-0ubuntu1) natty; urgency=low

  * New upstream release (LP: #660077)
    - The following patches were refreshed:
      + 0001-fix-release.patch
      + 0003-local-includes.patch
      + 0008-lp648900.patch: renamed as 0005-lp648900.patch
    - The following patches were dropped (included upstream):
      + 0005-lp601583.patch
      + 0006-network-interface-enumeration.patch
      + 0007-gnome-updates.patch
  * debian/patches/0006-testsuite-fixes.patch: testsuite fixes from head
    of 2.5 branch. These are needed for QRT and SRU testing (LP: #652211)
  * debian/patches/0007-honor-cflags.patch: have the parser makefile honor
    CFLAGS environment variable. Brings back missing symbols for the retracer
  * debian/patches/0008-lp652674.patch: fix warnings for messages without
    denied or requested masks (LP: #652674)
  * debian/apparmor.init: fix path to aa-status (LP: #654841)
  * debian/apport/source_apparmor.py: apport hook should use
    root_command_hook() for running apparmor_status (LP: #655529)
  * debian/apport/source_apparmor.py: use ProcKernelCmdline and don't clobber
    cmdline details (LP: #657091)
 -- Jamie Strandboge <email address hidden> Fri, 15 Oct 2010 12:23:00 -0500

Changed in apparmor (Ubuntu Natty):
status: Triaged → Fix Released
Revision history for this message
Martin Pitt (pitti) wrote : Please test proposed package

Accepted apparmor into maverick-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in apparmor (Ubuntu Maverick):
status: In Progress → Fix Committed
tags: added: verification-needed
Steve Beattie (sbeattie)
description: updated
Changed in apport (Ubuntu Lucid):
status: New → Won't Fix
Changed in apparmor (Ubuntu Lucid):
assignee: nobody → Jamie Strandboge (jdstrand)
importance: Undecided → Low
milestone: none → lucid-updates
status: New → In Progress
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Upgraded to 2.5.1-0ubuntu0.10.10.2 in two clean up to date VMs (amd64 and i386) and this issue is resolved.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apport - 1.15-0ubuntu1

---------------
apport (1.15-0ubuntu1) natty; urgency=low

  [ Martin Pitt ]
  * New upstream release. Changes since to our previous trunk snapshot:
    - Order symptom descriptions alphabetically. Thanks to Javier Collado.
    - Check $APPORT_SYMPTOMS_DIR environment variable for overriding the
      system default path. Thanks to Javier Collado.
    - testsuite: Check that crashdb.conf can have dynamic code to determine DB
      names and options.
    - ui.py test suite: Rewrite _gen_test_crash() to have the test process
      core dump itself, instead of using gdb to do it. The latter fails in
      ptrace restricted environments, such as Ubuntu 10.10.
    - launchpad.py: Use launchpadlib to file a bug instead of screen scraping.
      The latter was completely broken with current Launchpad, so this makes
      the test suite actually work again. Thanks to Diogo Matsubara!
    - launchpad.py: Change $APPORT_STAGING to $APPORT_LAUNCHPAD_INSTANCE, so
      that you can now specify "staging", "edge", or "dev" (for a local
      http://launchpad.dev installation). Thanks to Diogo Matsubara!
    - backends/packaging-apt-dpkg.py: Fix crash on empty lines in ProcMaps
      attachment.
    - doc/symptoms.txt: Fix typo, thanks Philip Muskovac. (LP: #590521)
    - apport/hookutils.py: rename ProcCmdLine to ProcKernelCmdLine to not wipe
      wipe out /proc/$pid/cmdline information. (LP: #657091)
    - apport/hookutils.py: attach_file() will not overwrite existing report
      keys, instead appending "_" until the key is unique.
    - Fix --save option to recognise ~, thanks Philip Muškovac. (LP: #657278)
    - Remove escalation_subscription from Ubuntu bug DB definition, turned out
      to not be useful; thanks Brian Murray.
    - launchpad.py: Fix APPORT_LAUNCHPAD_INSTANCE values with a https://
      prefix.
    - apt backend: Opportunistically try to install a -dbg package in addition
      to -dbgsym, to increase the chance that at least one of it exists.
      Thanks Daniel J Blueman!
  * debian/control: Switch Vcs-Bzr: to natty branch.

  [ Brian Murray ]
  * data/package-hooks/source_linux.py: Drop regression-potential tag. We are
    moving away from using regression-potential as a tag in the management of
    regression bug reports. Instead we will tag bugs regression-release and
    then create series, release, tasks for the release affected if the bug is
    in fact a regression.
 -- Martin Pitt <email address hidden> Fri, 12 Nov 2010 14:59:01 +0100

Changed in apport (Ubuntu Natty):
status: Triaged → Fix Released
Martin Pitt (pitti)
tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apparmor - 2.5.1-0ubuntu0.10.10.2

---------------
apparmor (2.5.1-0ubuntu0.10.10.2) maverick-proposed; urgency=low

  * New upstream release (LP: #660077)
    - The following patches were refreshed:
      + 0001-fix-release.patch
      + 0003-local-includes.patch
      + 0004-ubuntu-abstractions-updates.patch
      + 0008-lp648900.patch: renamed as 0005-lp648900.patch
    - The following patches were dropped (included upstream):
      + 0005-lp601583.patch
      + 0006-network-interface-enumeration.patch
      + 0007-gnome-updates.patch
  * debian/patches/0006-testsuite-fixes.patch: testsuite fixes from head
    of 2.5 branch. These are needed for QRT and SRU testing (LP: #652211)
  * debian/patches/0007-honor-cflags.patch: have the parser makefile honor
    CFLAGS environment variable. Brings back missing symbols for the retracer
  * debian/patches/0008-lp652674.patch: fix warnings for messages without
    denied or requested masks (LP: #652674)
  * debian/apparmor.init: fix path to aa-status (LP: #654841)
  * debian/apport/source_apparmor.py: apport hook should use
    root_command_hook() for running apparmor_status (LP: #655529)
  * debian/apport/source_apparmor.py: use ProcKernelCmdline and don't clobber
    cmdline details (LP: #657091)
  * debian/{rules,control}: move apache2 abstractions into the base package
    so we can put apache2 profiles into the -profiles package without
    aa-logprof bailing out. Patch by Marc Deslauriers.
    (LP: #539441)
  * debian/patches/0009-sensible-browser-pix.patch: use Pix with
    sensible-browser
  * debian/patches/0010-ubuntu-buildd.patch: skip parser caching test if
    the AppArmor securityfs introspection directory is not mounted, as
    is the case on Ubuntu buildds.
 -- Jamie Strandboge <email address hidden> Tue, 02 Nov 2010 12:04:06 -0500

Changed in apparmor (Ubuntu Maverick):
status: Fix Committed → Fix Released
Revision history for this message
Martin Pitt (pitti) wrote :

Accepted apparmor into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in apparmor (Ubuntu Lucid):
status: In Progress → Fix Committed
tags: removed: verification-done
tags: added: verification-needed
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Upgraded to 2.5.1-0ubuntu0.10.04.1 in lucid-proposed and this issue is resolved.

Martin Pitt (pitti)
tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (10.1 KiB)

This bug was fixed in the package apparmor - 2.5.1-0ubuntu0.10.04.1

---------------
apparmor (2.5.1-0ubuntu0.10.04.1) lucid-proposed; urgency=low

  * Backport 2.5.1-0ubuntu0.10.10.1 from maverick for userspace tools to work
    with newer kernels (LP: #660077)
    NOTE: user-tmp now uses 'owner' match, so non-default profiles will have
    to be adjusted when 2 separately confined applications that both use the
    user-tmp abstraction depend on being able to cooperatively share files
    with each other in /tmp or /var/tmp.
  * remove the following patches (features not appropriate for SRU):
    - 0002-add-chromium-browser.patch
    - 0003-local-includes.patch
    - 0004-ubuntu-abstractions-updates.patch
  * debian/rules (this makes it the same as what was shipped in 10.04 LTS
    release):
    - don't ship aa-update-browser and its man page (requires
      0004-ubuntu-abstractions-updates.patch)
    - don't ship apparmor.d/local/ (requires 0003-local-includes.patch)
    - don't use dh_apparmor (not in Ubuntu 10.04 LTS)
    - don't ship chromium profile
  * remove debian/profiles/chromium-browser
  * remove debian/aa-update-browser*
  * debian/apparmor-profiles.postinst: revert to that in lucid release
    (requires dh_apparmor and 0002-add-chromium-browser.patch)
  * remove debian/apparmor-profiles.postrm: doesn't make sense without
    0002-add-chromium-browser.patch
  * debian/control:
    - revert Build-Depends on debhelper (>= 5)
    - revert Standards-Version to 3.8.4
    - revert Vcs-Bzr
    - use Conflicts/Replaces version that was in Ubuntu 10.04 LTS
  * debian/patches/0011-lucid-compat-dbus.patch: move /var/lib/dbus/machine-id
    back into dbus, since profiles on 10.04 LTS expect it there
  * debian/patches/0012-lucid-compat-kde.patch: add kde4-config to kde
    abstraction, since the firefox profile on Ubuntu 10.04 LTS expects it to
    be there

apparmor (2.5.1-0ubuntu0.10.10.2) maverick-proposed; urgency=low

  * New upstream release (LP: #660077)
    - The following patches were refreshed:
      + 0001-fix-release.patch
      + 0003-local-includes.patch
      + 0004-ubuntu-abstractions-updates.patch
      + 0008-lp648900.patch: renamed as 0005-lp648900.patch
    - The following patches were dropped (included upstream):
      + 0005-lp601583.patch
      + 0006-network-interface-enumeration.patch
      + 0007-gnome-updates.patch
  * debian/patches/0006-testsuite-fixes.patch: testsuite fixes from head
    of 2.5 branch. These are needed for QRT and SRU testing (LP: #652211)
  * debian/patches/0007-honor-cflags.patch: have the parser makefile honor
    CFLAGS environment variable. Brings back missing symbols for the retracer
  * debian/patches/0008-lp652674.patch: fix warnings for messages without
    denied or requested masks (LP: #652674)
  * debian/apparmor.init: fix path to aa-status (LP: #654841)
  * debian/apport/source_apparmor.py: apport hook should use
    root_command_hook() for running apparmor_status (LP: #655529)
  * debian/apport/source_apparmor.py: use ProcKernelCmdline and don't clobber
    cmdline details (LP: #657091)
  * debian/{rules,control}: move apache2 abstractions into the base package
    so we can put ...

Changed in apparmor (Ubuntu Lucid):
status: Fix Committed → Fix Released
tags: added: testcase
Changed in apport (Ubuntu Maverick):
status: Fix Committed → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.