kill incorrectly parses negative PIDs

[dann frazier]

[Impact]
When kill is called with a negative argument, incorrect parsing can lead it to call sys_kill(-1), thus sending a signal to all permitted processes on the system. A couple of users have hit this while deploying Hadoop, which seems to tickle this - basically killing everything on the system.

[Test Case]
Though I don't know what Hadoop is calling, here's a couple of ways to trigger this:

One possibility is if kill were called w/ a numeric signal that
happened to start with a '1' and while omitting the required <pid>
argument:

kill -12

Another would be to specify a numeric signal (that again happened to
start with a 1) multiple times:
kill -13 -13 12345

[Regression Risk]
This is a backport from upstream that is already available in 16.10, with no known regressions.

[Andy Whitcroft (apw)]

Confirmed fixed in yakkety/zesty.

[Andy Whitcroft (apw)]

Hello dann, or anyone else affected,

Accepted procps into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/procps/2:3.3.10-4ubuntu2.1 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!

[dann frazier (dannf)]

I've verified the fix for this issue, but it appears to have exposed a bug with updating procps in an LXD container. Below I'll show the verification, followed by the update issue.

root@procps:~# apt-cache policy procps | grep Installed
  Installed: 2:3.3.10-4ubuntu2
root@procps:~# strace -f -o /tmp/strace.out su dannf -c '/bin/kill -12'
root@procps:~# grep sys_kill /tmp/strace.out
root@procps:~# grep kill /tmp/strace.out
3057 execve("/bin/su", ["su", "dannf", "-c", "/bin/kill -12"], [/* 11 vars */]) = 0
3062 execve("/bin/bash", ["bash", "-c", "/bin/kill -12"], [/* 17 vars */] <unfinished ...>
3062 execve("/bin/kill", ["/bin/kill", "-12"], [/* 17 vars */]) = 0
3062 kill(4294967295, SIGUSR2) = 0

(strace is interpreting the first arg as an unsigned int - it is -1 when signed.)

After the upgrade:
root@procps:~# strace -f -o /tmp/strace.out.new su dannf -c '/bin/kill -12'
root@procps:~# grep kill /tmp/strace.out.new
4241 execve("/bin/su", ["su", "dannf", "-c", "/bin/kill -12"], [/* 11 vars */]) = 0
4246 execve("/bin/bash", ["bash", "-c", "/bin/kill -12"], [/* 17 vars */] <unfinished ...>
4246 execve("/bin/kill", ["/bin/kill", "-12"], [/* 17 vars */]) = 0
4246 kill(-12, SIGUSR2) = -1 ESRCH (No such process)

However, here's the issue I observed upgrading in a container:

root@procps:~# apt install procps
Reading package lists... Done
Building dependency tree
Reading state information... Done
The following packages will be upgraded:
  procps
1 upgraded, 0 newly installed, 0 to remove and 18 not upgraded.
Need to get 208 kB of archives.
After this operation, 0 B of additional disk space will be used.
Get:1 http://ports.ubuntu.com/ubuntu-ports xenial-proposed/main arm64 procps arm64 2:3.3.10-4ubuntu2.1 [208 kB]
Fetched 208 kB in 0s (230 kB/s)
(Reading database ... 25398 files and directories currently installed.)
Preparing to unpack .../procps_2%3a3.3.10-4ubuntu2.1_arm64.deb ...
Unpacking procps (2:3.3.10-4ubuntu2.1) over (2:3.3.10-4ubuntu2) ...
Processing triggers for man-db (2.7.5-1) ...
Processing triggers for ureadahead (0.100.0-19) ...
Processing triggers for systemd (229-4ubuntu11) ...
Setting up procps (2:3.3.10-4ubuntu2.1) ...
update-rc.d: warning: start and stop actions are no longer supported; falling back to defaults
Job for systemd-sysctl.service failed because the control process exited with error code. See "systemctl status systemd-sysctl.service" and "journalctl -xe" for details.
invoke-rc.d: initscript procps, action "start" failed.
dpkg: error processing package procps (--configure):
 subprocess installed post-installation script returned error exit status 1
Errors were encountered while processing:
 procps
E: Sub-process /usr/bin/dpkg returned an error code (1)

[Martin Pitt (pitti)]

> systemd-sysctl.service failed

This is indeed a known wart in LXD containers, and it's conceptually difficult to fix (see bug 1576341 or https://github.com/lxc/lxcfs/issues/111 for some earlier discussion).

It would be wrong if sysctl would entirely ignore all failures, as this would make actual (unintended) failures/typos/etc. much harder to detect -- we don't want to hide these.

"apt-get install --reinstall procps" in lxd actually works fine in zesty, but not in xenial -- in xenial the postinst has "invoke-rc.d procps start" which is gone from zesty.

The current versions (also in Debian) do not start the init script on install/upgrades:

  https://anonscm.debian.org/cgit/collab-maint/procps.git/tree/debian/rules#n89

and I think that's the right thing -- this is usually something you want done at boot, but not during runtime, and it potentially breaks package upgrades too. So if you want to SRU this override_dh_install along, that would be fine with me (with my SRU hat on).

[Martin Pitt (pitti)]

Ah, this is already tracked in bug 1637300, nevermind.

[Brian Murray (brian-murray)]

Hello dann, or anyone else affected,

Accepted procps into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/procps/2:3.3.10-4ubuntu2.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!

[dann frazier (dannf)]

root@procpstest:~# strace -f -o /tmp/strace.out su dannf -c '/bin/kill -12'
root@procpstest:~# grep kill /tmp/strace.out
2623 execve("/bin/su", ["su", "dannf", "-c", "/bin/kill -12"], [/* 11 vars */]) = 0
2628 execve("/bin/bash", ["bash", "-c", "/bin/kill -12"], [/* 17 vars */]) = 0
2628 execve("/bin/kill", ["/bin/kill", "-12"], [/* 17 vars */]) = 0
2628 kill(-12, SIGUSR2) = -1 ESRCH (No such process)
root@procpstest:~#

[Launchpad Janitor (janitor)]

This bug was fixed in the package procps - 2:3.3.10-4ubuntu2.2

---------------
procps (2:3.3.10-4ubuntu2.2) xenial; urgency=medium

  * Don't start procps on install. This avoids errors on upgrade
    within a container. Backported from yakkety. (LP: #1637300)

procps (2:3.3.10-4ubuntu2.1) xenial; urgency=medium

  * Fix parsing of negative PIDs. (LP: #1637026)

 -- dann frazier <email address hidden> Wed, 26 Oct 2016 18:21:16 -0600

[Martin Pitt (pitti)]

The verification of the Stable Release Update for procps 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.