Skip to content

Commit 892f2de

Browse files
committed
fix systemd-notify when using a different PID namespace
The current support of systemd-notify has a race condition as the message send to the systemd notify socket might be dropped if the sender process is not running by the time systemd checks for the sender of the datagram. A proper fix of this in systemd would require changes to the kernel to maintain the cgroup of the sender process when it is dead (but it is not probably going to happen...) Generally, the solution to this issue is to specify the PID in the message itself so that systemd has not to guess the sender, but this wouldn't work when running in a PID namespace as the container will pass the PID known in its namespace (something like PID=1,2,3..) and systemd running on the host is not able to map it to the runc service. The proposed solution is to have a proxy in runc that forwards the messages to the host systemd. Example of this issue: projectatomic/atomic-system-containers#24 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
1 parent 1c9c074 commit 892f2de

File tree

5 files changed

+122
-16
lines changed

5 files changed

+122
-16
lines changed

notify_socket.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// +build linux
2+
3+
package main
4+
5+
import (
6+
"fmt"
7+
"net"
8+
"path/filepath"
9+
10+
"github.com/Sirupsen/logrus"
11+
"github.com/opencontainers/runtime-spec/specs-go"
12+
"github.com/urfave/cli"
13+
)
14+
15+
type notifySocket struct {
16+
socket *net.UnixConn
17+
host string
18+
socketPath string
19+
}
20+
21+
func newNotifySocket(context *cli.Context, notifySocketHost string, id string) *notifySocket {
22+
if notifySocketHost == "" {
23+
return nil
24+
}
25+
26+
root := filepath.Join(context.GlobalString("root"), id)
27+
path := filepath.Join(root, "notify.sock")
28+
29+
notifySocket := &notifySocket{
30+
socket: nil,
31+
host: notifySocketHost,
32+
socketPath: path,
33+
}
34+
35+
return notifySocket
36+
}
37+
38+
func (ns *notifySocket) Close() error {
39+
return ns.socket.Close()
40+
}
41+
42+
// If systemd is supporting sd_notify protocol, this function will add support
43+
// for sd_notify protocol from within the container.
44+
func (s *notifySocket) setupSpec(context *cli.Context, spec *specs.Spec) {
45+
mount := specs.Mount{Destination: s.host, Type: "bind", Source: s.socketPath, Options: []string{"bind"}}
46+
spec.Mounts = append(spec.Mounts, mount)
47+
spec.Process.Env = append(spec.Process.Env, fmt.Sprintf("NOTIFY_SOCKET=%s", s.host))
48+
}
49+
50+
func (s *notifySocket) setupSocket() error {
51+
addr := net.UnixAddr{
52+
Name: s.socketPath,
53+
Net: "unixgram",
54+
}
55+
56+
socket, err := net.ListenUnixgram("unixgram", &addr)
57+
if err != nil {
58+
return err
59+
}
60+
61+
s.socket = socket
62+
return nil
63+
}
64+
65+
func (notifySocket *notifySocket) run() {
66+
buf := make([]byte, 512)
67+
notifySocketHostAddr := net.UnixAddr{Name: notifySocket.host, Net: "unixgram"}
68+
client, err := net.DialUnix("unixgram", nil, &notifySocketHostAddr)
69+
if err != nil {
70+
logrus.Error(err)
71+
return
72+
}
73+
for {
74+
r, err := notifySocket.socket.Read(buf)
75+
if err != nil {
76+
break
77+
}
78+
79+
client.Write(buf[0:r])
80+
}
81+
}

restore.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,14 @@ func restoreContainer(context *cli.Context, spec *specs.Spec, config *configs.Co
165165
if err != nil {
166166
return -1, err
167167
}
168-
handler := newSignalHandler(!context.Bool("no-subreaper"))
168+
169+
notifySocket := newNotifySocket(context, os.Getenv("NOTIFY_SOCKET"), id)
170+
if notifySocket != nil {
171+
notifySocket.setupSpec(context, spec)
172+
notifySocket.setupSocket()
173+
}
174+
175+
handler := newSignalHandler(!context.Bool("no-subreaper"), notifySocket)
169176
if err := container.Restore(process, options); err != nil {
170177
return -1, err
171178
}

signals.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ const signalBufferSize = 2048
1717

1818
// newSignalHandler returns a signal handler for processing SIGCHLD and SIGWINCH signals
1919
// while still forwarding all other signals to the process.
20-
func newSignalHandler(enableSubreaper bool) *signalHandler {
20+
// If notifySocket is present, use it to read systemd notifications from the container and
21+
// forward them to notifySocketHost.
22+
func newSignalHandler(enableSubreaper bool, notifySocket *notifySocket) *signalHandler {
2123
if enableSubreaper {
2224
// set us as the subreaper before registering the signal handler for the container
2325
if err := system.SetSubreaper(1); err != nil {
@@ -30,7 +32,8 @@ func newSignalHandler(enableSubreaper bool) *signalHandler {
3032
// handle all signals for the process.
3133
signal.Notify(s)
3234
return &signalHandler{
33-
signals: s,
35+
signals: s,
36+
notifySocket: notifySocket,
3437
}
3538
}
3639

@@ -42,7 +45,8 @@ type exit struct {
4245
}
4346

4447
type signalHandler struct {
45-
signals chan os.Signal
48+
signals chan os.Signal
49+
notifySocket *notifySocket
4650
}
4751

4852
// forward handles the main signal event loop forwarding, resizing, or reaping depending
@@ -54,6 +58,11 @@ func (h *signalHandler) forward(process *libcontainer.Process, tty *tty) (int, e
5458
if err != nil {
5559
return -1, err
5660
}
61+
62+
if h.notifySocket != nil {
63+
go h.notifySocket.run()
64+
}
65+
5766
// perform the initial tty resize.
5867
tty.resize()
5968
for s := range h.signals {
@@ -75,6 +84,9 @@ func (h *signalHandler) forward(process *libcontainer.Process, tty *tty) (int, e
7584
// status because we must ensure that any of the go specific process
7685
// fun such as flushing pipes are complete before we return.
7786
process.Wait()
87+
if h.notifySocket != nil {
88+
h.notifySocket.Close()
89+
}
7890
return e.status, nil
7991
}
8092
}

utils.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,6 @@ func setupSpec(context *cli.Context) (*specs.Spec, error) {
5858
if err != nil {
5959
return nil, err
6060
}
61-
notifySocket := os.Getenv("NOTIFY_SOCKET")
62-
if notifySocket != "" {
63-
setupSdNotify(spec, notifySocket)
64-
}
6561
if os.Geteuid() != 0 {
6662
return nil, fmt.Errorf("runc should be run as root")
6763
}

utils_linux.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,6 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
9595
return lp, nil
9696
}
9797

98-
// If systemd is supporting sd_notify protocol, this function will add support
99-
// for sd_notify protocol from within the container.
100-
func setupSdNotify(spec *specs.Spec, notifySocket string) {
101-
spec.Mounts = append(spec.Mounts, specs.Mount{Destination: notifySocket, Type: "bind", Source: notifySocket, Options: []string{"bind"}})
102-
spec.Process.Env = append(spec.Process.Env, fmt.Sprintf("NOTIFY_SOCKET=%s", notifySocket))
103-
}
104-
10598
func destroy(container libcontainer.Container) {
10699
if err := container.Destroy(); err != nil {
107100
logrus.Error(err)
@@ -184,6 +177,7 @@ type runner struct {
184177
consoleSocket string
185178
container libcontainer.Container
186179
create bool
180+
notifySocket *notifySocket
187181
}
188182

189183
func (r *runner) terminalinfo() *libcontainer.TerminalInfo {
@@ -225,6 +219,10 @@ func (r *runner) run(config *specs.Process) (int, error) {
225219
r.destroy()
226220
return -1, fmt.Errorf("cannot use console socket if runc will not detach or allocate tty")
227221
}
222+
if detach && r.notifySocket != nil {
223+
r.destroy()
224+
return -1, fmt.Errorf("cannot detach when using NOTIFY_SOCKET")
225+
}
228226

229227
startFn := r.container.Start
230228
if !r.create {
@@ -233,7 +231,7 @@ func (r *runner) run(config *specs.Process) (int, error) {
233231
// Setting up IO is a two stage process. We need to modify process to deal
234232
// with detaching containers, and then we get a tty after the container has
235233
// started.
236-
handler := newSignalHandler(r.enableSubreaper)
234+
handler := newSignalHandler(r.enableSubreaper, r.notifySocket)
237235
tty, err := setupIO(process, rootuid, rootgid, config.Terminal, detach)
238236
if err != nil {
239237
r.destroy()
@@ -336,10 +334,21 @@ func startContainer(context *cli.Context, spec *specs.Spec, create bool) (int, e
336334
if id == "" {
337335
return -1, errEmptyID
338336
}
337+
338+
notifySocket := newNotifySocket(context, os.Getenv("NOTIFY_SOCKET"), id)
339+
if notifySocket != nil {
340+
notifySocket.setupSpec(context, spec)
341+
}
342+
339343
container, err := createContainer(context, id, spec)
340344
if err != nil {
341345
return -1, err
342346
}
347+
348+
if notifySocket != nil {
349+
notifySocket.setupSocket()
350+
}
351+
343352
// Support on-demand socket activation by passing file descriptors into the container init process.
344353
listenFDs := []*os.File{}
345354
if os.Getenv("LISTEN_FDS") != "" {
@@ -350,6 +359,7 @@ func startContainer(context *cli.Context, spec *specs.Spec, create bool) (int, e
350359
shouldDestroy: true,
351360
container: container,
352361
listenFDs: listenFDs,
362+
notifySocket: notifySocket,
353363
consoleSocket: context.String("console-socket"),
354364
detach: context.Bool("detach"),
355365
pidFile: context.String("pid-file"),

0 commit comments

Comments
 (0)