[libvirt] [PATCH v3 2/3] conf: add check if migration_host is a localhost address
Chen, Fan
chen.fan.fnst at cn.fujitsu.com
Thu Oct 9 01:54:14 UTC 2014
On Wed, 2014-10-08 at 12:33 +0200, Ján Tomko wrote:
> On 10/07/2014 06:07 AM, Chen Fan wrote:
> > Signed-off-by: Chen Fan <chen.fan.fnst at cn.fujitsu.com>
> > ---
> > src/libvirt_private.syms | 1 +
> > src/qemu/qemu_conf.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
> > src/qemu/qemu_conf.h | 2 ++
> > src/util/virsocketaddr.c | 24 +++++++++++++++++++++++
> > src/util/virsocketaddr.h | 2 ++
> > 5 files changed, 79 insertions(+)
> >
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 8ab1394..a104bc6 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -1911,6 +1911,7 @@ virSocketAddrGetIpPrefix;
> > virSocketAddrGetPort;
> > virSocketAddrGetRange;
> > virSocketAddrIsNetmask;
> > +virSocketAddrIsNumericLocalhost;
> > virSocketAddrIsPrivate;
> > virSocketAddrIsWildcard;
> > virSocketAddrMask;
> > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> > index adc6caf..6b0ac5c 100644
> > --- a/src/qemu/qemu_conf.c
> > +++ b/src/qemu/qemu_conf.c
> > @@ -707,6 +707,15 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> > GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox);
> >
> > GET_VALUE_STR("migration_host", cfg->migrateHost);
> > + if (cfg->migrateHost &&
> > + qemuCheckLocalhost(cfg->migrateHost)) {
> > + virReportError(VIR_ERR_CONF_SYNTAX,
> > + _("migration_host must not be the address of"
> > + " the local machine: %s"),
> > + cfg->migrateHost);
> > + goto cleanup;
> > + }
> > +
> > GET_VALUE_STR("migration_address", cfg->migrationAddress);
> >
> > GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp);
> > @@ -1371,3 +1380,44 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
> >
> > return qemuGetHugepagePath(&hugetlbfs[i]);
> > }
> > +
> > +bool
> > +qemuCheckLocalhost(const char *addrStr)
> > +{
> > + virSocketAddr addr;
> > + char *hostname, *tmp;
> > + bool encloseAddress = false;
> > + int family;
> > + bool ret = true;
> > +
> > + if (VIR_STRDUP(hostname, addrStr) < 0)
> > + return false;
> > +
> > + tmp = hostname;
> > +
> > + if (STRPREFIX(hostname, "[")) {
> > + char *end = strchr(hostname, ']');
> > + if (end) {
> > + *end = '\0';
> > + hostname++;
> > + encloseAddress = true;
> > + }
> > + }
>
> We don't format the qemu.conf back and we don't need the brackets for
> anything. We can just store the migration host without them in cfg->migrationHost
>
> > +
> > + if (STRPREFIX(hostname, "localhost"))
> > + goto cleanup;
> > +
> > + family = virSocketAddrNumericFamily(hostname);
> > + if ((family == AF_INET && !encloseAddress) ||
> > + family == AF_INET6) {
> > + if (virSocketAddrParse(&addr, hostname, family) > 0 &&
> > + virSocketAddrIsNumericLocalhost(&addr)) {
> > + goto cleanup;
> > + }
> > + }
>
> There's no need to check for family upfront.
>
> > +
> > + ret = false;
> > +cleanup:
> > + VIR_FREE(tmp);
> > + return ret;
> > +}
> > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> > index cb01fb6..c9ce53c 100644
> > --- a/src/qemu/qemu_conf.h
> > +++ b/src/qemu/qemu_conf.h
> > @@ -322,4 +322,6 @@ int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
> > char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage);
> > char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
> > size_t nhugetlbfs);
> > +
> > +bool qemuCheckLocalhost(const char *addrStr);
> > #endif /* __QEMUD_CONF_H */
> > diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
> > index 7fe7a15..6d36689 100644
> > --- a/src/util/virsocketaddr.c
> > +++ b/src/util/virsocketaddr.c
> > @@ -878,3 +878,27 @@ virSocketAddrNumericFamily(const char *address)
> > freeaddrinfo(res);
> > return family;
> > }
> > +
> > +/**
> > + * virSocketAddrIsNumericLocalhost:
> > + * @address: address to check
> > + *
> > + * Check if passed address is a numeric 'localhost' address.
> > + *
> > + * Returns: true if @address is a numeric 'localhost' address,
> > + * false otherwise
> > + */
> > +bool
> > +virSocketAddrIsNumericLocalhost(const virSocketAddr *addr)
>
> I've rewritten this function to take a 'const char *' argument.
> Along with the virStringStripIPv6Brackets function I've sent for review
> separately, this removes the need for a separate qemuCheckLocalhost function
> and it can be inlined.
>
> > +{
> > + struct in_addr tmp = { .s_addr = htonl(INADDR_LOOPBACK) };
> > + switch (addr->data.stor.ss_family) {
> > + case AF_INET:
> > + return memcmp(&addr->data.inet4.sin_addr.s_addr, &tmp.s_addr,
> > + sizeof(addr->data.inet4.sin_addr.s_addr)) == 0;
> > + case AF_INET6:
> > + return IN6_IS_ADDR_LOOPBACK(&addr->data.inet6.sin6_addr);
> > + }
> > + return false;
> > +
> > +}
>
> The diff I'll squash into this patch before pushing (after
> virStringStripIPv6Brackets is sorted out):
I had tested this diff and it works fine.
Thanks,
Chen
>
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -707,8 +707,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox);
>
> GET_VALUE_STR("migration_host", cfg->migrateHost);
> + virStringStripIPv6Brackets(cfg->migrateHost);
> if (cfg->migrateHost &&
> - qemuCheckLocalhost(cfg->migrateHost)) {
> + (STRPREFIX(cfg->migrateHost, "localhost") ||
> + virSocketAddrIsNumericLocalhost(cfg->migrateHost))) {
> virReportError(VIR_ERR_CONF_SYNTAX,
> _("migration_host must not be the address of"
> " the local machine: %s"),
> @@ -1380,44 +1382,3 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
>
> return qemuGetHugepagePath(&hugetlbfs[i]);
> }
> -
> -bool
> -qemuCheckLocalhost(const char *addrStr)
> -{
> - virSocketAddr addr;
> - char *hostname, *tmp;
> - bool encloseAddress = false;
> - int family;
> - bool ret = true;
> -
> - if (VIR_STRDUP(hostname, addrStr) < 0)
> - return false;
> -
> - tmp = hostname;
> -
> - if (STRPREFIX(hostname, "[")) {
> - char *end = strchr(hostname, ']');
> - if (end) {
> - *end = '\0';
> - hostname++;
> - encloseAddress = true;
> - }
> - }
> -
> - if (STRPREFIX(hostname, "localhost"))
> - goto cleanup;
> -
> - family = virSocketAddrNumericFamily(hostname);
> - if ((family == AF_INET && !encloseAddress) ||
> - family == AF_INET6) {
> - if (virSocketAddrParse(&addr, hostname, family) > 0 &&
> - virSocketAddrIsNumericLocalhost(&addr)) {
> - goto cleanup;
> - }
> - }
> -
> - ret = false;
> -cleanup:
> - VIR_FREE(tmp);
> - return ret;
> -}
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index c9ce53c..cb01fb6 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -322,6 +322,4 @@ int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
> char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage);
> char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
> size_t nhugetlbfs);
> -
> -bool qemuCheckLocalhost(const char *addrStr);
> #endif /* __QEMUD_CONF_H */
>
> diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
> index 6d36689..a19e3af 100644
> --- a/src/util/virsocketaddr.c
> +++ b/src/util/virsocketaddr.c
> @@ -889,15 +889,24 @@ virSocketAddrNumericFamily(const char *address)
> * false otherwise
> */
> bool
> -virSocketAddrIsNumericLocalhost(const virSocketAddr *addr)
> +virSocketAddrIsNumericLocalhost(const char *addr)
> {
> + struct addrinfo *res;
> struct in_addr tmp = { .s_addr = htonl(INADDR_LOOPBACK) };
> - switch (addr->data.stor.ss_family) {
> + struct sockaddr_in *inet4;
> + struct sockaddr_in6 *inet6;
> +
> + if (virSocketAddrParseInternal(&res, addr, AF_UNSPEC, false) < 0)
> + return false;
> +
> + switch (res->ai_addr->sa_family) {
> case AF_INET:
> - return memcmp(&addr->data.inet4.sin_addr.s_addr, &tmp.s_addr,
> - sizeof(addr->data.inet4.sin_addr.s_addr)) == 0;
> + inet4 = (struct sockaddr_in*) res->ai_addr;
> + return memcmp(&inet4->sin_addr.s_addr, &tmp.s_addr,
> + sizeof(inet4->sin_addr.s_addr)) == 0;
> case AF_INET6:
> - return IN6_IS_ADDR_LOOPBACK(&addr->data.inet6.sin6_addr);
> + inet6 = (struct sockaddr_in6*) res->ai_addr;
> + return IN6_IS_ADDR_LOOPBACK(&(inet6->sin6_addr));
> }
> return false;
>
> diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
> index fa9e98b..053855b 100644
> --- a/src/util/virsocketaddr.h
> +++ b/src/util/virsocketaddr.h
> @@ -127,5 +127,5 @@ bool virSocketAddrIsWildcard(const virSocketAddr *addr);
>
> int virSocketAddrNumericFamily(const char *address);
>
> -bool virSocketAddrIsNumericLocalhost(const virSocketAddr *addr);
> +bool virSocketAddrIsNumericLocalhost(const char *addr);
> #endif /* __VIR_SOCKETADDR_H__ */
>
> Jan
>
More information about the libvir-list
mailing list