killall with 65 arguments kills more than expected

Bug #1507681 reported by Gregory Boyce on 2015-10-19
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
psmisc (Ubuntu)
Medium
Unassigned
Precise
Medium
Christian Ehrhardt 

Bug Description

[Impact]

 * killall with exactly 65 (33 in 32-bit environments) arguments can kill random processes
 * this can be accidentially or even maliciously used to kill processes
 * root casue is an off-by-one error

[Test Case]

 * as seen in the bug description above, but please note that this triggers the bug only sometimes (1/3 of my tries)
   ps xa | wc -l
   for i in `seq 1 65`; do touch ~/tmp_tasks/test$i; done;
   for i in `seq 1 65`; do echo ~/tmp_tasks/test$i; done | xargs killall
   ps xa | wc -l

[Regression Potential]

 * there should be no/minimal regression Potential
   - the fix itself is minimal
   - no solution (other than maybe exploits) should rely on this behaviour

[Original Report]
killall in Precise is supposed to limit the number of arguments to 64, but due to a fencepost error, 66 arguments will be blocked but 65 is not.

With 65 arguments, the behavior varies, but in some cases will send a signal to random processes.

# ps xa | wc -l
164

# mkdir ~/tmp_tasks/
# for i in `seq 1 65`; do touch ~/tmp_tasks/test$i; done;

# for i in `seq 1 65`; do echo ~/tmp_tasks/test$i; done | xargs killall
Connection to 198.18.88.176 closed by remote host.
Connection to 198.18.88.176 closed.

# ps xa | wc -l
126

This is fixed upstream and at the very least trusty works fine.

Gregory Boyce (gregory-boyce) wrote :

attached is a proposed fix

The attachment "killall-with-65-arguments-kills-all-proce.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

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

tags: added: patch
Robie Basak (racb) on 2015-10-20
Changed in psmisc (Ubuntu Precise):
status: New → Triaged
importance: Undecided → Medium
Changed in psmisc (Ubuntu):
status: New → Fix Released
tags: added: bitesize
Robie Basak (racb) on 2016-05-24
Changed in psmisc (Ubuntu Precise):
assignee: nobody → ChristianEhrhardt (paelzer)

FYI - the number it crashes is 32 on 32 bit platforms.
I found the upstream fix and prepped an SRU based on it.
I also integrated the associated man page change , but not all the printf changes it has (it is an SRU, so minimum changes)

Also I have to note that the reproduction steps above only triggered it in 1 of 3 of my cases.
But then the error is clear just looking at the code.

Adding debdiff and the SRU template now ...

SRU-Template:

[Impact]

 * killall with exactly 65 (33 in 32-bit environments) arguments can kill random processes
 * this can be accidentially or even maliciously used to kill processes
 * root casue is an off-by-one error

[Test Case]

 * as seen in the bug description above, but please note that this triggers the bug only sometimes (1/3 of my tries)
   ps xa | wc -l
   for i in `seq 1 65`; do touch ~/tmp_tasks/test$i; done;
   for i in `seq 1 65`; do echo ~/tmp_tasks/test$i; done | xargs killall
   ps xa | wc -l

[Regression Potential]

 * there should be no/minimal regression Potential
   - the fix itself is minimal
   - no solution (other than maybe exploits) should rely on this behaviour

Changed in psmisc (Ubuntu Precise):
status: Triaged → In Progress
Brian Murray (brian-murray) wrote :

I've uploaded this to the queue for review by the SRU team.

description: updated

Hello Gregory, or anyone else affected,

Accepted psmisc into precise-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/psmisc/22.15-2ubuntu1.2 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 psmisc (Ubuntu Precise):
status: In Progress → Fix Committed
tags: added: verification-needed
Changed in psmisc (Ubuntu):
importance: Undecided → Medium

Unfortunately no 3rd party verification yet - adding one of my own.

Since I wasn't sure about the reproducibility I went a bit parallel:
Using the test (very slightly) modified in 4 precise containers I got a reliable trigger:
2/4 directly
2/4 after one retry

mkdir -p ~/tmp_tasks; ps xa | wc -l; for i in `seq 1 65`; do touch ~/tmp_tasks/test$i; done; for i in `seq 1 65`; do echo ~/tmp_tasks/test$i; done | xargs killall; ps xa | wc -l

Upgrading to proposed got them all reliable to report:
"Maximum number of names is 64"

I played a bit with ps and couldn't find any other regressions that would be obvious.
That said - setting verified.

tags: added: verification-done-precise
removed: verification-needed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package psmisc - 22.15-2ubuntu1.2

---------------
psmisc (22.15-2ubuntu1.2) precise; urgency=low

  * Backport of upstream fix:
    - Fix killall with 65 arguments (LP: #1507681)

 -- Christian Ehrhardt <email address hidden> Tue, 07 Jun 2016 11:36:17 -0700

Changed in psmisc (Ubuntu Precise):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for psmisc has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers