SIGSEGV when file2str reads zero bytes

Bug #1242746 reported by John-Mark Bell on 2013-10-21
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
procps (Ubuntu)
Medium
Dave Chiluk
Precise
Undecided
Dave Chiluk
Quantal
Undecided
Dave Chiluk
Raring
Undecided
Dave Chiluk
Saucy
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

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?

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!

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
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) on 2013-10-28
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)
Dave Chiluk (chiluk) wrote :
Dave Chiluk (chiluk) wrote :
Dave Chiluk (chiluk) wrote :
Dave Chiluk (chiluk) wrote :
Dave Chiluk (chiluk) wrote :

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

Looking for some SRU lovin now.

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) on 2013-10-28
description: updated
Brian Murray (brian-murray) wrote :

I'll take care of sponsoring this.

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
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
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
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!

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) on 2013-11-04
Changed in procps (Ubuntu Saucy):
assignee: nobody → Dave Chiluk (chiluk)
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
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.

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
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  Edit
Everyone can see this information.

Other bug subscribers