Skip to content

Commit

Permalink
runtime: handle signal 34 for musl setgid
Browse files Browse the repository at this point in the history
It has been observed that setgid hangs when using cgo with musl.
This fix ensures that signal 34 gets handled in an appropriate way,
like signal 33 when using glibc.

Fixes #39343

Change-Id: I89565663e2c361f62cbccfe80aaedf290bd58d57
Reviewed-on: https://go-review.googlesource.com/c/go/+/236518
Run-TryBot: Tobias Klauser <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Tobias Klauser <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
  • Loading branch information
GeorgeTsilias authored and ianlancetaylor committed Oct 28, 2020
1 parent 40d1ec5 commit 49b017f
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/runtime/sigtab_linux_generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var sigtable = [...]sigTabT{
/* 31 */ {_SigThrow, "SIGSYS: bad system call"},
/* 32 */ {_SigSetStack + _SigUnblock, "signal 32"}, /* SIGCANCEL; see issue 6997 */
/* 33 */ {_SigSetStack + _SigUnblock, "signal 33"}, /* SIGSETXID; see issues 3871, 9400, 12498 */
/* 34 */ {_SigNotify, "signal 34"},
/* 34 */ {_SigSetStack + _SigUnblock, "signal 34"}, /* musl SIGSYNCCALL; see issue 39343 */
/* 35 */ {_SigNotify, "signal 35"},
/* 36 */ {_SigNotify, "signal 36"},
/* 37 */ {_SigNotify, "signal 37"},
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/sigtab_linux_mipsx.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var sigtable = [...]sigTabT{
/* 31 */ {_SigNotify, "SIGXFSZ: file size limit exceeded"},
/* 32 */ {_SigSetStack + _SigUnblock, "signal 32"}, /* SIGCANCEL; see issue 6997 */
/* 33 */ {_SigSetStack + _SigUnblock, "signal 33"}, /* SIGSETXID; see issues 3871, 9400, 12498 */
/* 34 */ {_SigNotify, "signal 34"},
/* 34 */ {_SigSetStack + _SigUnblock, "signal 34"}, /* musl SIGSYNCCALL; see issue 39343 */
/* 35 */ {_SigNotify, "signal 35"},
/* 36 */ {_SigNotify, "signal 36"},
/* 37 */ {_SigNotify, "signal 37"},
Expand Down

3 comments on commit 49b017f

@richfelker
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit was an incorrect fix. Use of kernel-level signal 34 here is not an interface contract of musl. It's a runtime-variable property determined by SIGRTMIN/SIGRTMAX. The underlying issue seems to be use of raw syscalls or passing raw self-generated sigset_t values rather than using the libc signal API.

@ianlancetaylor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use Gerrit for code review, so very few people see comments made on GitHub commits. The place to make this comment is https://golang.org/cl/236518. Thanks.

Note that Go on GNU/Linux explicitly does not use the libc API.

@richfelker
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

musl is not GNU/Linux. :-)

Please sign in to comment.