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 <vuori(a)notcom.org> wrote: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: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.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 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 <vuori(a)notcom.org> wrote:Hi, thanks for the fast reply. On Thu, Apr 13, 2023 at 01:25:05PM +0200, Stefano Brivio wrote:Oh, I didn't know it would be so specific. I'm not sure then -- your call.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.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.Sure, patch posted. -- StefanoI 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.