11:44 a.m.
Ensure that the socket credentials we got back on BSD are valid before
using them.
---
src/rpc/virnetsocket.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b311aae..49c6ddc 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1166,6 +1166,18 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
return -1;
}
+ if (cr.cr_version != XUCRED_VERSION) {
+ virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Failed to get valid client socket identity"));
+ return -1;
+ }
+
+ if (cr.cr_ngroups == 0) {
+ virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Failed to get valid client socket identity
groups"));
+ return -1;
+ }
+
*pid = -1;
*uid = cr.cr_uid;
*gid = cr.cr_gid;
--
1.8.1.5
11:44 a.m.
New subject: [libvirt] [PATCH 2/3] BSD: Ensure process creation timestamp is init'd
While BSDs don't support process creation timestamp information via
PEERCRED for Unix sockets, we need to actually initialize the value
because it is used by the libvirt code.
---
src/rpc/virnetsocket.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 49c6ddc..152c5fc 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1153,7 +1153,7 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
uid_t *uid,
gid_t *gid,
pid_t *pid,
- unsigned long long *timestamp ATTRIBUTE_UNUSED)
+ unsigned long long *timestamp)
{
struct xucred cr;
socklen_t cr_len = sizeof(cr);
@@ -1178,7 +1178,9 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
return -1;
}
+ /* PID and process creation time are not supported on BSDs */
*pid = -1;
+ *timestamp = -1;
*uid = cr.cr_uid;
*gid = cr.cr_gid;
--
1.8.1.5
11:17 a.m.
New subject: [libvirt] [PATCH 2/3] BSD: Ensure process creation timestamp is init'd
On Tue, Sep 24, 2013 at 11:44:55AM -0500, Doug Goldstein wrote:
While BSDs don't support process creation timestamp information
via
PEERCRED for Unix sockets, we need to actually initialize the value
because it is used by the libvirt code.
---
src/rpc/virnetsocket.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 49c6ddc..152c5fc 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1153,7 +1153,7 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
uid_t *uid,
gid_t *gid,
pid_t *pid,
- unsigned long long *timestamp ATTRIBUTE_UNUSED)
+ unsigned long long *timestamp)
{
struct xucred cr;
socklen_t cr_len = sizeof(cr);
@@ -1178,7 +1178,9 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
return -1;
}
+ /* PID and process creation time are not supported on BSDs */
*pid = -1;
+ *timestamp = -1;
*uid = cr.cr_uid;
*gid = cr.cr_gid;
ACK
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 :|
11:44 a.m.
New subject: [libvirt] [PATCH 3/3] event: Make debug message match function comments
The debug message said there was a timeout of 0 pending for -1 ms which
made me think this is where a hang was coming from but according to the
function comments this case means that there is no timeout pending so
make the debug message say that instead of saying there's a -1 ms
timeout.
---
src/util/vireventpoll.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index c84ac20..8a4c8bc 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -358,7 +358,10 @@ static int virEventPollCalculateTimeout(int *timeout) {
*timeout = -1;
}
- EVENT_DEBUG("Timeout at %llu due in %d ms", then, *timeout);
+ if (*timeout > -1)
+ EVENT_DEBUG("Timeout at %llu due in %d ms", then, *timeout);
+ else
+ EVENT_DEBUG("%s", "No timeout is pending");
return 0;
}
--
1.8.1.5
11:17 a.m.
New subject: [libvirt] [PATCH 3/3] event: Make debug message match function comments
On Tue, Sep 24, 2013 at 11:44:56AM -0500, Doug Goldstein wrote:
The debug message said there was a timeout of 0 pending for -1 ms
which
made me think this is where a hang was coming from but according to the
function comments this case means that there is no timeout pending so
make the debug message say that instead of saying there's a -1 ms
timeout.
---
src/util/vireventpoll.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index c84ac20..8a4c8bc 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -358,7 +358,10 @@ static int virEventPollCalculateTimeout(int *timeout) {
*timeout = -1;
}
- EVENT_DEBUG("Timeout at %llu due in %d ms", then, *timeout);
+ if (*timeout > -1)
+ EVENT_DEBUG("Timeout at %llu due in %d ms", then, *timeout);
+ else
+ EVENT_DEBUG("%s", "No timeout is pending");
return 0;
}
ACK
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 :|
11:14 a.m.
On Tue, Sep 24, 2013 at 11:44 AM, Doug Goldstein <cardoe(a)cardoe.com> wrote:
Ensure that the socket credentials we got back on BSD are valid
before
using them.
---
src/rpc/virnetsocket.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b311aae..49c6ddc 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1166,6 +1166,18 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
return -1;
}
+ if (cr.cr_version != XUCRED_VERSION) {
+ virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Failed to get valid client socket identity"));
+ return -1;
+ }
+
+ if (cr.cr_ngroups == 0) {
+ virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Failed to get valid client socket identity
groups"));
+ return -1;
+ }
+
*pid = -1;
*uid = cr.cr_uid;
*gid = cr.cr_gid;
--
1.8.1.5
Ping this patch. Justin Clift ran into a bug related with this when
testing 1.1.3 rc1. I've looked for other projects doing similar checks
and Wayland uses this behavior on the BSDs. Always makes me feel good
when I don't come up with a unique solution and instead other projects
do it as well.
--
Doug Goldstein
11:16 a.m.
On Tue, Sep 24, 2013 at 11:44:54AM -0500, Doug Goldstein wrote:
Ensure that the socket credentials we got back on BSD are valid
before
using them.
---
src/rpc/virnetsocket.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b311aae..49c6ddc 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1166,6 +1166,18 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
return -1;
}
+ if (cr.cr_version != XUCRED_VERSION) {
+ virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Failed to get valid client socket identity"));
+ return -1;
+ }
+
+ if (cr.cr_ngroups == 0) {
+ virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
+ _("Failed to get valid client socket identity
groups"));
+ return -1;
+ }
+
*pid = -1;
*uid = cr.cr_uid;
*gid = cr.cr_gid;
ACK,
Though presumably this still won't make things work, as we'll now get a fatal
error reported, rather than silently using bogus data ?
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 :|
11:19 a.m.
On Fri, Sep 27, 2013 at 11:16 AM, Daniel P. Berrange
<berrange(a)redhat.com> wrote:
On Tue, Sep 24, 2013 at 11:44:54AM -0500, Doug Goldstein wrote:
> Ensure that the socket credentials we got back on BSD are valid before
> using them.
> ---
> src/rpc/virnetsocket.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index b311aae..49c6ddc 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -1166,6 +1166,18 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
> return -1;
> }
>
> + if (cr.cr_version != XUCRED_VERSION) {
> + virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
> + _("Failed to get valid client socket identity"));
> + return -1;
> + }
> +
> + if (cr.cr_ngroups == 0) {
> + virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
> + _("Failed to get valid client socket identity
groups"));
> + return -1;
> + }
> +
> *pid = -1;
> *uid = cr.cr_uid;
> *gid = cr.cr_gid;
ACK,
Though presumably this still won't make things work, as we'll now get a fatal
error reported, rather than silently using bogus data ?
Exactly. Hoping to get some free time to play on my Mac mini next week
to solve the actual underlying error.
--
Doug Goldstein
4075
days inactive
4078
days old
7 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Doug Goldstein