* passt.c: incorrect signal() return value check
@ 2023-04-13 10:24 Valtteri Vuorikoski
2023-04-13 11:25 ` Stefano Brivio
0 siblings, 1 reply; 4+ messages in thread
From: Valtteri Vuorikoski @ 2023-04-13 10:24 UTC (permalink / raw)
To: passt-dev
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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: passt.c: incorrect signal() return value check
2023-04-13 10:24 passt.c: incorrect signal() return value check Valtteri Vuorikoski
@ 2023-04-13 11:25 ` Stefano Brivio
2023-04-13 14:47 ` Valtteri Vuorikoski
0 siblings, 1 reply; 4+ messages in thread
From: Stefano Brivio @ 2023-04-13 11:25 UTC (permalink / raw)
To: Valtteri Vuorikoski; +Cc: passt-dev, David Gibson
Hi Valtteri,
On Thu, 13 Apr 2023 13:24:34 +0300
Valtteri Vuorikoski <vuori@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: passt.c: incorrect signal() return value check
2023-04-13 11:25 ` Stefano Brivio
@ 2023-04-13 14:47 ` Valtteri Vuorikoski
2023-04-13 17:46 ` Stefano Brivio
0 siblings, 1 reply; 4+ messages in thread
From: Valtteri Vuorikoski @ 2023-04-13 14:47 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, David Gibson
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: passt.c: incorrect signal() return value check
2023-04-13 14:47 ` Valtteri Vuorikoski
@ 2023-04-13 17:46 ` Stefano Brivio
0 siblings, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2023-04-13 17:46 UTC (permalink / raw)
To: Valtteri Vuorikoski; +Cc: passt-dev, David Gibson
On Thu, 13 Apr 2023 17:47:27 +0300
Valtteri Vuorikoski <vuori@notcom.org> wrote:
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-13 17:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-13 10:24 passt.c: incorrect signal() return value check Valtteri Vuorikoski
2023-04-13 11:25 ` Stefano Brivio
2023-04-13 14:47 ` Valtteri Vuorikoski
2023-04-13 17:46 ` Stefano Brivio
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).