SIGSEGV when file2str reads zero bytes

Bug #1242746 reported by John-Mark Bell
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
procps (Ubuntu)
Fix Released
Medium
Dave Chiluk
Precise
Fix Released
Undecided
Dave Chiluk
Quantal
Fix Released
Undecided
Dave Chiluk
Raring
Fix Released
Undecided
Dave Chiluk
Saucy
Fix Released
Undecided
Dave Chiluk

Bug Description

[Impact]

 * various procps utilities crashing with a SIGSEGV if the file2str function in
   proc/readproc.c when it reads zero bytes

 * This is a regression introduced with (LP: #1150413)

 * The upload checks for zero length reads and now returns -1 instead of 0, like it used
   to.

[Test Case]

 * This does not happen often enough to create a real testcase, but some users have
   reported that repeatedly running top has occassionally exhibited this problem.

[Regression Potential]

 * Minimal, as this reverts file2str to original behavior in the case of zero length
   reads.

[Other Info]

 * This patch is backported from upstream

_________________________________________________________________________________________
The changes made in the following commit, which backported a number of changes to the procps package in precise, result in various procps utilities crashing with a SIGSEGV if the file2str function in proc/readproc.c reads zero bytes.

http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/precise/procps/precise-updates/revision/61

This corresponds to the following procps package version: 1:3.2.8-11ubuntu6.1

Prior to this changeset, file2str would return -1 if the read failed; now it does not, which is not expected by other parts of the procps codebase, hence the crash.

Upstream have fixed this issue in the following commit:

https://www.gitorious.org/procps/procps/commit/526bc5dfa924177e68be0123bd67e3370955f924

Revision history for this message
Dave Gilbert (ubuntu-treblig) wrote :

Triaged: Pointer to upstream fix

John-Mark: How often does this trigger, is this something that causes regular pain or a rare issue?

Revision history for this message
John-Mark Bell (jmb202) wrote :

Dave: I've been seeing this about twice a day since I picked up the new packages (about a week ago).

I've got a cron job that runs every 5 minutes, and one of the things it runs is "top -c -b -n 2 -d 2", which is what's exploding for me.

The systems this job runs on tend to be transient (they're autodeployed systems in my regression test network), so would rarely exist for a prolonged period of time.

So, to answer your question: it clearly doesn't happen every time, but happens sufficiently often to cause me pain!

Revision history for this message
Dave Gilbert (ubuntu-treblig) wrote :

Triaged: (as stated but didn't press the button) pointer to upstream fix
Medium: Rare to hit it, but it's procps so it's core, and if it's going to affect lots of things it's going to be a pain

Changed in procps (Ubuntu):
importance: Undecided → Medium
status: New → Triaged
tags: added: regression-update
Revision history for this message
Steve Langasek (vorlon) wrote :

Dave, please follow up on this regression reported against the procps SRU.

Changed in procps (Ubuntu):
assignee: nobody → Dave Chiluk (chiluk)
Dave Chiluk (chiluk)
Changed in procps (Ubuntu):
status: Triaged → In Progress
Changed in procps (Ubuntu Precise):
status: New → In Progress
Changed in procps (Ubuntu Quantal):
status: New → In Progress
Changed in procps (Ubuntu Raring):
status: New → In Progress
Changed in procps (Ubuntu Precise):
assignee: nobody → Dave Chiluk (chiluk)
Changed in procps (Ubuntu Quantal):
assignee: nobody → Dave Chiluk (chiluk)
Changed in procps (Ubuntu Raring):
assignee: nobody → Dave Chiluk (chiluk)
Revision history for this message
Dave Chiluk (chiluk) wrote :
Revision history for this message
Dave Chiluk (chiluk) wrote :
Revision history for this message
Dave Chiluk (chiluk) wrote :
Revision history for this message
Dave Chiluk (chiluk) wrote :
Revision history for this message
Dave Chiluk (chiluk) wrote :

All debdiff's attached. Trusty already has this patch.

Looking for some SRU lovin now.

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "lp1242746.precise.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Dave Chiluk (chiluk)
description: updated
Revision history for this message
Brian Murray (brian-murray) wrote :

I'll take care of sponsoring this.

Revision history for this message
Adam Conrad (adconrad) wrote : Please test proposed package

Hello John-Mark, or anyone else affected,

Accepted procps into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/procps/1:3.2.8-11ubuntu6.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in procps (Ubuntu):
status: In Progress → Fix Released
Changed in procps (Ubuntu Precise):
status: In Progress → Fix Committed
tags: added: verification-needed
Changed in procps (Ubuntu Raring):
status: In Progress → Fix Committed
Revision history for this message
Adam Conrad (adconrad) wrote :

Hello John-Mark, or anyone else affected,

Accepted procps into raring-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/procps/1:3.3.3-2ubuntu5.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in procps (Ubuntu Quantal):
status: In Progress → Fix Committed
Revision history for this message
Adam Conrad (adconrad) wrote :

Hello John-Mark, or anyone else affected,

Accepted procps into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/procps/1:3.3.3-2ubuntu3.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in procps (Ubuntu Saucy):
status: New → Fix Committed
Revision history for this message
Adam Conrad (adconrad) wrote :

Hello John-Mark, or anyone else affected,

Accepted procps into saucy-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/procps/1:3.3.3-2ubuntu9 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Revision history for this message
John-Mark Bell (jmb202) wrote :

I've tested this on Precise by running the attached script.

With the old package version (1:3.2.8-11ubuntu6.1) this caused top to crash after about 90 minutes.
With the new package version (1:3.2.8-11ubuntu6.3), it's been running without incident for the last 24 hours.

Note, that as we're dealing with a race between top reading the contents of proc and processes being spawned/destroyed, it's tricky to reproduce in a controlled manner, so I went for the brute-force approach.

Nothing else appears untoward with the new version.

tags: added: verification-done
removed: verification-needed
tags: added: verification-done-precise verification-needed
removed: verification-done
Dave Chiluk (chiluk)
Changed in procps (Ubuntu Saucy):
assignee: nobody → Dave Chiluk (chiluk)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package procps - 1:3.2.8-11ubuntu6.3

---------------
procps (1:3.2.8-11ubuntu6.3) precise; urgency=low

  * Avoid SEGV if file2str should read zero bytes. This is a backport of
    526bc5df from upstream. When utility buffers were introduced for file2str
    read requests, a subtle change was inadvertently introduced such that a
    read of zero no longer returns a -1 value. This returns to the behavior to
    returning -1 on zero byte reads. (LP: #1242746)
 -- Dave Chiluk <email address hidden> Mon, 28 Oct 2013 10:56:11 -0700

Changed in procps (Ubuntu Precise):
status: Fix Committed → Fix Released
Revision history for this message
Dave Chiluk (chiluk) wrote :

I am verifying p q r s today with the John-Mark's script. I'll complete verification tomorrow assuming is succeeds.

Revision history for this message
Dave Chiluk (chiluk) wrote :

All are happily chugging away. Verified p,q,r,s.

tags: added: verification-done-quantal verification-done-raring verification-done-saucy
Changed in procps (Ubuntu Quantal):
status: Fix Committed → Fix Released
Changed in procps (Ubuntu Raring):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package procps - 1:3.3.3-2ubuntu9

---------------
procps (1:3.3.3-2ubuntu9) saucy; urgency=low

  * Avoid SEGV if file2str should read zero bytes. This is a backport of
    526bc5df from upstream. When utility buffers were introduced for file2str
    read requests, a subtle change was inadvertently introduced such that a
    read of zero no longer returns a -1 value. This returns to the behavior to
    returning -1 on zero byte reads. (LP: #1242746)
 -- Dave Chiluk <email address hidden> Mon, 28 Oct 2013 10:36:11 -0700

Changed in procps (Ubuntu Saucy):
status: Fix Committed → Fix Released
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.