exec'ing a setuid binary from a threaded program sometimes fails to setuid
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Linux |
Confirmed
|
High
|
|||
golang-1.6 (Ubuntu) |
Invalid
|
Undecided
|
Unassigned | ||
Xenial |
Fix Released
|
High
|
Michael Hudson-Doyle | ||
Yakkety |
Invalid
|
Undecided
|
Unassigned | ||
Zesty |
Invalid
|
Undecided
|
Unassigned | ||
linux (Ubuntu) |
Fix Released
|
High
|
Colin Ian King | ||
Xenial |
Fix Released
|
High
|
Colin Ian King | ||
Yakkety |
Fix Released
|
High
|
Colin Ian King | ||
Zesty |
Fix Released
|
High
|
Colin Ian King |
Bug Description
== SRU template for golang-1.6 ==
[Impact]
The kernel bug reported below means that occasionally (maybe 1 in 1000 times) the snapd -> snap-confine exec that is part of a snap execution fails to take the setuid bit on the snap-confine binary into account which means that the execution fails. This is extremely confusing for the user of the snap who just sees a permission denied error with no explanation.
The kernel bug has been fixed in Xenial+ but not all users of snapd are on xenial+ kernels (they might be on trusty or another distribution entirely).
Backporting this fix will mean that the snapd in the core snap will get the workaround next time it is built and because the snapd in trusty or the other distro will re-exec into the snapd in the core snap before execing snap-confine, users should not see the above behaviour.
[Test case]
This will be a bit tricky as the kernel bug has been fixed. A xenial container on a trusty host/VM should do the trick. The test case from https:/
[Regression potential]
If there is a bug in the patch it could cause deadlocks in currently working programs. But the patch is pretty simple and has passed review upstream so I think it should be OK.
== SRU REQUEST XENIAL, YAKKETY, ZESTY ==
Due to two race conditions in check_unsafe_
== Fix ==
Sauce patch for Xenial, Yakkety + Zesty:
https:/
This fix re-executes the unsafe check if there is a discrepancy between the expected fs count and the found count during the racy window during thread exec or exit. This re-check occurs very infrequently and saves a lot of addition locking on per thread structures that would make performance of fork/exec/exit prohibitively expensive.
== Test case ==
See the example C code in the patch, https:/
Run the test code as follows: for i in $(seq 1000); do ./a; done
With the patch, no messages are emitted, without the patch, one sees a message:
"Failed, got euid 1000 (expecting 0)"
..which shows the setuid program failed the check_unsafe_exec() because of the race.
== Regression potential ==
breaking existing safe exec semantics.
=======
This can be reproduced with
https:/
With that, and go 1.8, if you run “make” and then
for i in `seq 99`; do ./a_go; done
you'll see a variable number of ”GOT 1000” (or whatever your user id is). If you don't, add one or two more 9s on there.
That's a simple go reproducer. You can also use “a_p” instead of “a_go” to see one that only uses pthreads. “a_c” is a C version that does *not* reproduce the issue.
But it's not pthreads: if in a_go.go you comment out the “import "C"”, you'll still see the “GOT 1000” messages, in a static binary that uses no pthreads, just clone(2). You'll also see a bunch of warnings because it's not properly handling an EAGAIN from clone, but that's unrelated.
If you pin the process to a single thread using taskset, you don't get the issue from a_go; a_p continues to reproduce the issue. In some virtualized environments we haven't been able to reproduce the issue either (e.g. some aws instances), but kvm works (you need -smp to see the issue from a_go).
ProblemType: Bug
DistroRelease: Ubuntu 16.04
Package: linux-image-
ProcVersionSign
Uname: Linux 4.4.0-64-generic x86_64
NonfreeKernelMo
ApportVersion: 2.20.1-0ubuntu2.5
Architecture: amd64
AudioDevicesInUse:
USER PID ACCESS COMMAND
/dev/snd/pcmC0D0p: john 2354 F...m pulseaudio
/dev/snd/
CurrentDesktop: Unity
Date: Tue Mar 14 17:17:23 2017
HibernationDevice: RESUME=
InstallationDate: Installed on 2014-04-27 (1051 days ago)
InstallationMedia: Ubuntu 14.04 LTS "Trusty Tahr" - Release amd64 (20140417)
MachineType: Dell Inc. Latitude E6510
ProcFB: 0 inteldrmfb
ProcKernelCmdLine: BOOT_IMAGE=
RelatedPackageV
linux-
linux-
linux-firmware 1.157.8
SourcePackage: linux
SystemImageInfo: Error: command ['system-
UpgradeStatus: Upgraded to xenial on 2015-06-18 (634 days ago)
dmi.bios.date: 12/05/2013
dmi.bios.vendor: Dell Inc.
dmi.bios.version: A16
dmi.board.vendor: Dell Inc.
dmi.chassis.type: 9
dmi.chassis.vendor: Dell Inc.
dmi.modalias: dmi:bvnDellInc.
dmi.product.name: Latitude E6510
dmi.product.
dmi.sys.vendor: Dell Inc.
Changed in linux (Ubuntu): | |
status: | New → Triaged |
importance: | Undecided → High |
Changed in linux (Ubuntu Xenial): | |
status: | New → Triaged |
importance: | Undecided → High |
tags: | added: kernel-key |
tags: |
added: kernel-da-key removed: kernel-key |
tags: |
added: kernel-key removed: kernel-da-key |
Changed in linux (Ubuntu Xenial): | |
assignee: | nobody → Colin Ian King (colin-king) |
status: | Triaged → In Progress |
tags: |
added: kernel-da-key removed: kernel-key |
Changed in linux (Ubuntu Xenial): | |
status: | Incomplete → In Progress |
description: | updated |
Changed in linux (Ubuntu): | |
status: | Triaged → Fix Committed |
Changed in linux (Ubuntu Zesty): | |
status: | New → Fix Committed |
Changed in linux (Ubuntu Yakkety): | |
status: | New → Fix Committed |
Changed in linux (Ubuntu Xenial): | |
status: | In Progress → Fix Committed |
Changed in linux (Ubuntu Yakkety): | |
assignee: | nobody → Colin Ian King (colin-king) |
Changed in linux (Ubuntu Zesty): | |
assignee: | nobody → Colin Ian King (colin-king) |
importance: | Undecided → High |
Changed in linux (Ubuntu Yakkety): | |
importance: | Undecided → High |
Changed in linux (Ubuntu): | |
assignee: | nobody → Colin Ian King (colin-king) |
Changed in linux (Ubuntu): | |
status: | Fix Committed → Fix Released |
Changed in golang-1.6 (Ubuntu): | |
status: | New → Invalid |
Changed in golang-1.6 (Ubuntu Yakkety): | |
status: | New → Invalid |
Changed in golang-1.6 (Ubuntu Zesty): | |
status: | New → Invalid |
Changed in golang-1.6 (Ubuntu Xenial): | |
assignee: | nobody → Michael Hudson-Doyle (mwhudson) |
importance: | Undecided → High |
description: | updated |
Changed in golang-1.6 (Ubuntu Xenial): | |
status: | New → In Progress |
Changed in linux: | |
importance: | Unknown → High |
status: | Unknown → Confirmed |
I also tried this in 4.10.0-11-generic, same results.