passt.c: incorrect signal() return value check
In current master, passt.c:main() has an incorrect check on signal() return value: if (sigaction(SIGCHLD, &sa, NULL) || signal(SIGPIPE, SIG_IGN)) die("Couldn't install signal handlers"); signal() returns SIG_ERR on failure or the previous setting if present. If a setting has been inherited from the parent (as is the case when starting from systemd with the default setting of IgnoreSIGPIPE=yes), the check will fail. The if statement should check for SIG_ERR for the SIGPIPE case, or alternatively switch to sigaction() since that's used for everything else in the code. -Valtteri (please cc on replies)
Hi Valtteri,
On Thu, 13 Apr 2023 13:24:34 +0300
Valtteri Vuorikoski
In current master, passt.c:main() has an incorrect check on signal() return value:
if (sigaction(SIGCHLD, &sa, NULL) || signal(SIGPIPE, SIG_IGN)) die("Couldn't install signal handlers");
signal() returns SIG_ERR on failure or the previous setting if present. If a setting has been inherited from the parent (as is the case when starting from systemd with the default setting of IgnoreSIGPIPE=yes), the check will fail.
Right, thanks for reporting this. By the way, feel free to share your systemd unit files, I guess we could add them under contrib/ in case they are useful for somebody.
The if statement should check for SIG_ERR for the SIGPIPE case, or alternatively switch to sigaction() since that's used for everything else in the code.
I guess we could just check SIG_ERR from signal(), it looks a bit simpler, and perhaps split this into two cases (a failure on signal() doesn't mean we "[c]ouldn't install signal handlers"). Will you post a patch? It's trivial, so if it's unnecessary effort for you I can also go ahead and do it.
-Valtteri (please cc on replies)
-- Stefano
Hi, thanks for the fast reply. On Thu, Apr 13, 2023 at 01:25:05PM +0200, Stefano Brivio wrote:
signal() returns SIG_ERR on failure or the previous setting if present. If a setting has been inherited from the parent (as is the case when starting from systemd with the default setting of IgnoreSIGPIPE=yes), the check will fail.
Right, thanks for reporting this.
By the way, feel free to share your systemd unit files, I guess we could add them under contrib/ in case they are useful for somebody.
I'll see if I can put together some kind of readable version for a patch. This is a somewhat intricate setup to get a specific constellation of painful legacy services running with 1.x series Podman under systemd, but the "1.x series Podman under systemd" part might be useful to someone I guess.
I guess we could just check SIG_ERR from signal(), it looks a bit simpler, and perhaps split this into two cases (a failure on signal() doesn't mean we "[c]ouldn't install signal handlers").
Will you post a patch? It's trivial, so if it's unnecessary effort for you I can also go ahead and do it.
If you can just push the "== SIG_ERR" fix please do, my only tree is currently on another machine. -Valtteri
On Thu, 13 Apr 2023 17:47:27 +0300
Valtteri Vuorikoski
Hi, thanks for the fast reply.
On Thu, Apr 13, 2023 at 01:25:05PM +0200, Stefano Brivio wrote:
signal() returns SIG_ERR on failure or the previous setting if present. If a setting has been inherited from the parent (as is the case when starting from systemd with the default setting of IgnoreSIGPIPE=yes), the check will fail.
Right, thanks for reporting this.
By the way, feel free to share your systemd unit files, I guess we could add them under contrib/ in case they are useful for somebody.
I'll see if I can put together some kind of readable version for a patch. This is a somewhat intricate setup to get a specific constellation of painful legacy services running with 1.x series Podman under systemd, but the "1.x series Podman under systemd" part might be useful to someone I guess.
Oh, I didn't know it would be so specific. I'm not sure then -- your call.
I guess we could just check SIG_ERR from signal(), it looks a bit simpler, and perhaps split this into two cases (a failure on signal() doesn't mean we "[c]ouldn't install signal handlers").
Will you post a patch? It's trivial, so if it's unnecessary effort for you I can also go ahead and do it.
If you can just push the "== SIG_ERR" fix please do, my only tree is currently on another machine.
Sure, patch posted. -- Stefano
participants (2)
-
Stefano Brivio
-
Valtteri Vuorikoski