[libvirt] [PATCH] systemd: directly notify systemd instead of using sd_notify

[Daniel P. Berrange]

On Mon, Jun 06, 2016 at 04:46:50PM +0200, Peter Krempa wrote:
> On Mon, Jun 06, 2016 at 15:16:50 +0100, Daniel Berrange wrote:
> > The sd_notify method is used to tell systemd when libvirtd
> > has finished starting up. All it does is send a datagram
> > containing the string parameter to systemd on a UNIX socket
> > named in the NOTIFY_SOCKET environment variable. Rather than
> > pulling in the systemd libraries for this, just code the
> > notification directly in libvirt as this is a stable ABI
> > from systemd's POV which explicitly allows independant
> > implementations:
> > 
> > See "Reimplementable Independently" column in the
> > "$NOTIFY_SOCKET Daemon Notifications" row:
> > 
> > https://www.freedesktop.org/wiki/Software/systemd/InterfacePortabilityAndStabilityChart/
> > 
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> >  configure.ac              |  2 --
> >  libvirt.spec.in           | 12 -----------
> >  m4/virt-systemd-daemon.m4 | 34 -------------------------------
> >  src/Makefile.am           |  4 ++--
> >  src/util/virsystemd.c     | 52 ++++++++++++++++++++++++++++++++++++++++++-----
> >  5 files changed, 49 insertions(+), 55 deletions(-)
> >  delete mode 100644 m4/virt-systemd-daemon.m4
> 
> [...]
> 
> > diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
> > index 4883f94..71b8f33 100644
> > --- a/src/util/virsystemd.c
> > +++ b/src/util/virsystemd.c
> 
> [...]
> 
> > @@ -480,9 +482,49 @@ int virSystemdTerminateMachine(const char *name)
> >  void
> >  virSystemdNotifyStartup(void)
> >  {
> > -#ifdef WITH_SYSTEMD_DAEMON
> > -    sd_notify(0, "READY=1");
> > -#endif
> > +#ifdef HAVE_SYS_UN_H
> > +    const char *path;
> > +    const char *msg = "READY=1";
> > +    int fd;
> > +    struct sockaddr_un un = {
> > +        .sun_family = AF_UNIX,
> > +    };
> > +    struct iovec iov = {
> > +        .iov_base = (char *)msg,
> > +        .iov_len = strlen(msg),
> > +    };
> > +    struct msghdr mh = {
> > +        .msg_name = &un,
> > +        .msg_namelen = sizeof(un),
> > +        .msg_iov = &iov,
> > +        .msg_iovlen = 1,
> > +    };
> > +
> > +    if (!(path = virGetEnvBlockSUID("NOTIFY_SOCKET"))) {
> > +        VIR_DEBUG("Skipping systemd notify, not requested");
> > +        return;
> > +    }
> > +
> > +    if (strlen(path) > sizeof(un.sun_path)) {
> 
> Off-by-one by not considering the trailing \0

UNIX socket addresses are *not* NUL-terminated strings. The full
108 bytes in the 'sun_path' field are considered to be the path.
So even if you do have a NUL in your string, everything following
it is also considered part of the address - its why its important
to ensure the sun_path is set to all-zeros :-)

> > +        VIR_WARN("Systemd notify socket path '%s' too long", path);
> > +        return;
> > +    }
> 
> ACK

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|