[systemd-devel] [PATCH] core/service: check if mainpid matches only if it set

[Zbigniew Jędrzejewski-Szmek]

On Fri, Feb 14, 2014 at 02:38:48AM +0100, Lennart Poettering wrote:
> On Fri, 14.02.14 02:24, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> 
> > 
> > On Fri, Feb 14, 2014 at 02:07:36AM +0100, Lennart Poettering wrote:
> > > On Mon, 30.12.13 17:26, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> > > 
> > > >  
> > > > -        if (s->notify_access == NOTIFY_MAIN && pid != s->main_pid) {
> > > > +        if (s->notify_access == NOTIFY_MAIN && s->main_pid != 0 && pid != s->main_pid) {
> > > >                  log_warning_unit(u->id,
> > > >                                   "%s: Got notification message from PID %lu, but reception only permitted for PID %lu",
> > > >                                   u->id, (unsigned long) pid, (unsigned long) s->main_pid);
> > > >                  return;
> > > 
> > > Hmm, this doesn't look right. This is about access control after all,
> > > and we shouldn't allow these updates to be accepted from just anybody,
> > > just because we don't know the main pid...
> > > 
> > > I have now changed this so that if we don't know the main pid, we will
> > > print this at a lower log level (debug) and be less confusing with PID
> > > == 0...
> > > 
> > > Does that make sense?
> > No, I'm afraid that this actually exacerbates the original problem:
> > in the bug report httpd.service/start times out because the notification
> > is rejected. With your change it will be ignored silently (unless debugging
> > is turned on, of course).
> > 
> > I'm not convinced that accepting updates from "anyone" until MAINPID is known
> > is bad. What about adding a note in the manpage:
> > 
> >   Note: initially systemd will accept notification from any process
> >   inside the service.  Make sure that systemd has been notified about
> >   the value of $MAINPID before starting any untrusted processes.
> 
> So, what I am afraid of here is that some process has some unpriviliged
> children (CGI scripts or so), and for some reason they stay around after
> the daemon itself is already terminated, and then can confuse
> things... I mean, let's say somebody sets KillMode=none, and thus
> processes can survive all the way into the next process start, and can
> then quickly confuse systemd that they are the real main pid or so (for
> example, by sending MAINPID= or so...). Or even if we consider the
> KillMode=none thing irrelevant, then there might still be a small window
> where the main pid is already dead and we kill the other processes and
> wait for them and they then send us invalid data?
In the bug report it seems that this happens during startup...
Other cases where I saw this were also during startup, and KillMode
was default, as far as I remember. Even if $MAINPID has died, we should
be able to distinguish this from the case where it hasn't been set yet.

Zbyszek