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

[Zbigniew Jędrzejewski-Szmek]

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.

?

Zbyszek