[PATCH] device: avoid signals sent to the mbim-proxy process

[Ben Chan]

On Mon, Aug 7, 2017 at 1:27 PM, Aleksander Morgado <aleksander at aleksander.es
> wrote:

> On Mon, Aug 7, 2017 at 10:09 PM, Dan Williams <dcbw at redhat.com> wrote:
> > On Mon, 2017-08-07 at 11:58 -0700, Ben Chan wrote:
> >>
> >>
> >> On Mon, Aug 7, 2017 at 8:58 AM, Dan Williams <dcbw at redhat.com> wrote:
> >> > On Sun, 2017-08-06 at 13:13 +0200, Aleksander Morgado wrote:
> >> > > If e.g. mbim-proxy is started by ModemManager and we send a
> >> > Ctrl+C to
> >> > > it, the signal would be propagated to the mbim-proxy process and
> >> > we
> >> > > would kill it right away, while leaving ModemManager still around
> >> > > wondering why the socket to the proxy got a HUP.
> >> > >
> >> > > Avoid this, by making sure the mbim-proxy gets its own process
> >> > group.
> >> >
> >>
> >> lgtm
> >>
> >> > The way you've done it here is AFAIK the standard way to detach a
> >> > child
> >> > process and make it standalone.  We've done that for years in most
> >> > of
> >> > the things NetworkManager spawns, for example (VPN daemons, DHCP
> >> > daemon, avahi-autoipd, dnsmasq, teamd, etc).
> >> >
> >> > Though instead of (0,0) NM uses:
> >> >
> >> >         pid_t pid;
> >> >
> >> >         pid = getpid ();
> >> >         setpgid (pid, pid);
> >> >
> >> > Which I think is a bit more idiomatic.
> >>
> >> Isn't that equivalent to `setpgid (0, 0)`? Is `setgpid (pid, pid)`
> >> preferred for some reason?
> >
> > It should get called after the fork(), so it'll get the child's pid.
> > Maybe you read getuid() instead (which since MM is root *would* be 0?)
> >
>
> I believe they're equivalent, doc says:
>
> If pid is zero, then the process ID of the calling process is used.
> If pgid is zero, then the PGID of the process specified by pid is made
> the same as its process ID. So both 0 is really the same as getpid()
> in both.
>

That's what I think too.  Given that `spawn_child_setup` is run on the
child process before exec(), getpid() returns the child pid. `setpgid (0,
0)` should be equivalent to `setpgid (getpid(), getpid())` in such
scenario. Unless there is some benefit to split the call into getpid +
setpgid, `setpgid (0, 0)` seems more preferable.


>
> --
> Aleksander
> https://aleksander.es
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/libmbim-devel/attachments/20170807/b2dcecf5/attachment.html>