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 :|