On Sun, Oct 13, 2013 at 08:04:25AM +0100, Brian Candler wrote:
> I was debugging libvirt with OSX today, and got as far as finding
> the problem with LOCAL_PEERCRED, then googled this only to find that
> Ryota Ozaki had fixed the problems a few days ago!
>
> However you still may find the following patch useful. It tightens
> up the checking in the LOCAL_PEERCRED block, and in particular fixes
> the unlocking of the socket in the error return path for invalid
> groups, by using the same logic from SO_PEERCRED - have a 'goto
> cleanup' in all return paths.
>
> (Detail: I found that when getsockopt was being called with
> SOL_SOCKET, cr_ngroups was typically <0, probably because it was
> uninitialised. However once the check for this was tightened, it
> hung because the socket wasn't being unlocked on return. So better
> to (a) initialise it to a negative value anyway, and (b) fix the
> return path)
>
> However I have not checked that NGROUPS is defined on other BSD-like
> systems. You could just have "if (cr.cr_ngroups <= 0)" instead.
>
> Regards,
>
> Brian Candler.
>
> --- src/rpc/virnetsocket.c.orig 2013-10-10 22:37:49.000000000 +0100
> +++ src/rpc/virnetsocket.c 2013-10-12 22:51:57.000000000 +0100
> @@ -1157,8 +1157,10 @@
> {
> struct xucred cr;
> socklen_t cr_len = sizeof(cr);
> + int ret = -1;
> virObjectLock(sock);
>
> + cr.cr_ngroups = -1;
> # if defined(__APPLE__)
> if (getsockopt(sock->fd, SOL_LOCAL, LOCAL_PEERCRED, &cr,
> &cr_len) < 0) {
> # else
> @@ -1166,20 +1168,19 @@
> # endif
> virReportSystemError(errno, "%s",
> _("Failed to get client socket identity"));
> - virObjectUnlock(sock);
> - return -1;
> + goto cleanup;
> }
>
> if (cr.cr_version != XUCRED_VERSION) {
> virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
> _("Failed to get valid client socket identity"));
> - return -1;
> + goto cleanup;
> }
>
> - if (cr.cr_ngroups == 0) {
> + if (cr.cr_ngroups <= 0 || cr.cr_ngroups > NGROUPS) {
> virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
> _("Failed to get valid client socket
> identity groups"));
> - return -1;
> + goto cleanup;
> }
>
> /* PID and process creation time are not supported on BSDs */
> @@ -1188,8 +1189,11 @@
> *uid = cr.cr_uid;
> *gid = cr.cr_gid;
>
> + ret = 0;
> +
> +cleanup:
> virObjectUnlock(sock);
> - return 0;
> + return ret;
> }
> #else
> int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,
Unfortunately your patch does not apply since your mail client has
messed up line wrapping. Also there have been conflicting changes
to the code since your patch. I would fix it myself, but I don't
have ability to compile test code on BSD platforms.
Can you update your patch & re-send.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list