killall with 65 arguments kills more than expected

[Gregory Boyce]

[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)]

attached is a proposed fix

[Ubuntu Foundations Team Bug Bot (crichton)]

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

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

[ChristianEhrhardt (paelzer)]

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

[Brian Murray (brian-murray)]

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

[Adam Conrad (adconrad)]

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!

[ChristianEhrhardt (paelzer)]

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.

[Launchpad Janitor (janitor)]

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

[Martin Pitt (pitti)]

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.