[libvirt] [PATCH] Fix possible invalid read in adminClientGetInfo
Erik Skultety
eskultet at redhat.com
Wed Jun 29 08:06:03 UTC 2016
On 29/06/16 07:10, Ján Tomko wrote:
> virNetServerClientGetInfo returns the client's remote address
> as a string, which is a part of the client object.
>
> Use VIR_STRDUP to make a copy which can be freely accessed
> even after the virNetServerClient object is unlocked.
Although you're right, it was a bit difficult to see what kind of bug
are referring to with this fix, but now I see it...however, would you
mind adding the reproducer to the commit message (for future purposes)? :)
> ---
> daemon/admin_server.c | 3 ++-
> src/rpc/virnetserverclient.c | 8 ++++++--
> src/rpc/virnetserverclient.h | 2 +-
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/daemon/admin_server.c b/daemon/admin_server.c
> index cb9079c..9f24f68 100644
> --- a/daemon/admin_server.c
> +++ b/daemon/admin_server.c
> @@ -221,7 +221,7 @@ adminClientGetInfo(virNetServerClientPtr client,
> int ret = -1;
> int maxparams = 0;
> bool readonly;
> - const char *sock_addr = NULL;
> + char *sock_addr = NULL;
> const char *attr = NULL;
> virTypedParameterPtr tmpparams = NULL;
> virIdentityPtr identity = NULL;
> @@ -300,6 +300,7 @@ adminClientGetInfo(virNetServerClientPtr client,
>
> cleanup:
> virObjectUnref(identity);
> + VIR_FREE(sock_addr);
> return ret;
> }
>
> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index 39fe860..81da82c 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -1606,20 +1606,24 @@ virNetServerClientGetTransport(virNetServerClientPtr client)
>
> int
> virNetServerClientGetInfo(virNetServerClientPtr client,
> - bool *readonly, const char **sock_addr,
> + bool *readonly, char **sock_addr,
> virIdentityPtr *identity)
> {
> int ret = -1;
> + const char *addr;
>
> virObjectLock(client);
> *readonly = client->readonly;
>
> - if (!(*sock_addr = virNetServerClientRemoteAddrStringURI(client))) {
> + if (!(addr = virNetServerClientRemoteAddrStringURI(client))) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("No network socket associated with client"));
> goto cleanup;
> }
>
> + if (VIR_STRDUP(*sock_addr, addr) < 0)
> + goto cleanup;
> +
> if (!client->identity) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("No identity information available for client"));
> diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
> index c243a68..a53cc00 100644
> --- a/src/rpc/virnetserverclient.h
> +++ b/src/rpc/virnetserverclient.h
> @@ -149,7 +149,7 @@ bool virNetServerClientNeedAuth(virNetServerClientPtr client);
>
> int virNetServerClientGetTransport(virNetServerClientPtr client);
> int virNetServerClientGetInfo(virNetServerClientPtr client,
> - bool *readonly, const char **sock_addr,
> + bool *readonly, char **sock_addr,
> virIdentityPtr *identity);
>
> #endif /* __VIR_NET_SERVER_CLIENT_H__ */
>
Good catch, ACK.
Erik
More information about the libvir-list
mailing list