<p>laforge has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/17802">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">lib/netns: Fix up error paths<br><br>The error handling in the code was doing exactly what one would not<br>expect. If we switch to a netns and then encounter an error, we<br>obviously have to switch back to the original netns before returning.<br><br>Likewise, if we temporarily change the signal mask, we need to switch<br>back to the original one before returning.<br><br>Change-Id: I9ff5ae7bffc5bd7629dae0af1b72cfea548f9039<br>---<br>M lib/netns.c<br>1 file changed, 54 insertions(+), 20 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-ggsn refs/changes/02/17802/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/lib/netns.c b/lib/netns.c</span><br><span>index 43fca82..8d26330 100644</span><br><span>--- a/lib/netns.c</span><br><span>+++ b/lib/netns.c</span><br><span>@@ -1,5 +1,6 @@</span><br><span> /*</span><br><span> * Copyright (C) 2014-2017, Travelping GmbH <info@travelping.com></span><br><span style="color: hsl(120, 100%, 40%);">+ * Copyright (C) 2020, Harald Welte <laforge@gnumonks.org></span><br><span> *</span><br><span> * This program is free software: you can redistribute it and/or modify</span><br><span> * it under the terms of the GNU Affero General Public License as</span><br><span>@@ -61,8 +62,11 @@</span><br><span> if ((rc = sigprocmask(SIG_BLOCK, &intmask, oldmask)) != 0)</span><br><span> return -rc;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (setns(nsfd, CLONE_NEWNET) < 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (setns(nsfd, CLONE_NEWNET) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* restore old mask if we couldn't switch the netns */</span><br><span style="color: hsl(120, 100%, 40%);">+ sigprocmask(SIG_SETMASK, oldmask, NULL);</span><br><span> return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>@@ -84,7 +88,8 @@</span><br><span> int open_ns(int nsfd, const char *pathname, int flags)</span><br><span> {</span><br><span> sigset_t intmask, oldmask;</span><br><span style="color: hsl(0, 100%, 40%);">- int fd;</span><br><span style="color: hsl(120, 100%, 40%);">+ int ret;</span><br><span style="color: hsl(120, 100%, 40%);">+ int fd = -1;</span><br><span> int rc;</span><br><span> </span><br><span> /* mask off all signals, store old signal mask */</span><br><span>@@ -94,23 +99,34 @@</span><br><span> return -rc;</span><br><span> </span><br><span> /* associate the calling thread with namespace file descriptor */</span><br><span style="color: hsl(0, 100%, 40%);">- if (setns(nsfd, CLONE_NEWNET) < 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (setns(nsfd, CLONE_NEWNET) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto restore_sigmask;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> /* open the requested file/path */</span><br><span style="color: hsl(0, 100%, 40%);">- if ((fd = open(pathname, flags)) < 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((fd = open(pathname, flags)) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto restore_defaultns;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = fd;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+restore_defaultns:</span><br><span> /* return back to default namespace */</span><br><span> if (setns(default_nsfd, CLONE_NEWNET) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- close(fd);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (fd >= 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ close(fd);</span><br><span> return -errno;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+restore_sigmask:</span><br><span> /* restore process mask */</span><br><span> if ((rc = sigprocmask(SIG_SETMASK, &oldmask, NULL)) != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- close(fd);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (fd >= 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ close(fd);</span><br><span> return -rc;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- return fd;</span><br><span style="color: hsl(120, 100%, 40%);">+ return ret;</span><br><span> }</span><br><span> </span><br><span> /*! create a socket in another namespace.</span><br><span>@@ -124,7 +140,8 @@</span><br><span> int socket_ns(int nsfd, int domain, int type, int protocol)</span><br><span> {</span><br><span> sigset_t intmask, oldmask;</span><br><span style="color: hsl(0, 100%, 40%);">- int sk;</span><br><span style="color: hsl(120, 100%, 40%);">+ int ret;</span><br><span style="color: hsl(120, 100%, 40%);">+ int sk = -1;</span><br><span> int rc;</span><br><span> </span><br><span> /* mask off all signals, store old signal mask */</span><br><span>@@ -134,25 +151,34 @@</span><br><span> return -rc;</span><br><span> </span><br><span> /* associate the calling thread with namespace file descriptor */</span><br><span style="color: hsl(0, 100%, 40%);">- if (setns(nsfd, CLONE_NEWNET) < 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (setns(nsfd, CLONE_NEWNET) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto restore_sigmask;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> /* create socket of requested domain/type/proto */</span><br><span style="color: hsl(0, 100%, 40%);">- if ((sk = socket(domain, type, protocol)) < 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((sk = socket(domain, type, protocol)) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto restore_defaultns;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = sk;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+restore_defaultns:</span><br><span> /* return back to default namespace */</span><br><span> if (setns(default_nsfd, CLONE_NEWNET) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- close(sk);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sk >= 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ close(sk);</span><br><span> return -errno;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+restore_sigmask:</span><br><span> /* restore process mask */</span><br><span> if ((rc = sigprocmask(SIG_SETMASK, &oldmask, NULL)) != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- close(sk);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sk >= 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ close(sk);</span><br><span> return -rc;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- return sk;</span><br><span style="color: hsl(120, 100%, 40%);">+ return ret;</span><br><span> }</span><br><span> </span><br><span> /*! initialize this network namespace helper module.</span><br><span>@@ -172,6 +198,7 @@</span><br><span> * \returns File descriptor of network namespace; negative errno in case of error */</span><br><span> int get_nsfd(const char *name)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ int ret = 0;</span><br><span> int rc;</span><br><span> int fd;</span><br><span> sigset_t intmask, oldmask;</span><br><span>@@ -203,19 +230,26 @@</span><br><span> return -rc;</span><br><span> </span><br><span> /* create a new network namespace */</span><br><span style="color: hsl(0, 100%, 40%);">- if (unshare(CLONE_NEWNET) < 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (unshare(CLONE_NEWNET) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto restore_sigmask;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> if (mount("/proc/self/ns/net", path, "none", MS_BIND, NULL) < 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = -errno;</span><br><span> </span><br><span> /* switch back to default namespace */</span><br><span> if (setns(default_nsfd, CLONE_NEWNET) < 0)</span><br><span> return -errno;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+restore_sigmask:</span><br><span> /* restore process mask */</span><br><span> if ((rc = sigprocmask(SIG_SETMASK, &oldmask, NULL)) != 0)</span><br><span> return -rc;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* might have been set above in case mount fails */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ret < 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ return ret;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* finally, open the created namespace file descriptor from default ns */</span><br><span> if ((fd = open(path, O_RDONLY)) < 0)</span><br><span> return -errno;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/17802">change 17802</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-ggsn/+/17802"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-ggsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I9ff5ae7bffc5bd7629dae0af1b72cfea548f9039 </div>
<div style="display:none"> Gerrit-Change-Number: 17802 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>