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

[Dan Williams]

On Mon, 2017-08-07 at 13:38 -0700, Ben Chan wrote:
> 
> 
> On Mon, Aug 7, 2017 at 1:27 PM, Aleksander Morgado <aleksander at aleksa
> nder.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.

Yep, you're right.  0,0 seems fine then.

Dan