Re: [libvirt] [PATCH] virnetsocket: fix getsockopt on FreeBSD

On Sun, Nov 03, 2013 at 05:18:17PM -0600, Doug Goldstein wrote: > On Sat, Nov 2, 2013 at 11:20 AM, Ryota Ozaki <ozaki.ryota(a)gmail.com&gt; wrote: > > ping? > > > > Hope it's in the next release. It fixes libvirtd to accept virsh > > accesses on localhost. > > > > ozaki-r > > > > On Fri, Oct 25, 2013 at 12:48 AM, Ryota Ozaki <ozaki.ryota(a)gmail.com&gt; wrote: > >> aa0f099 introduced a strict error checking for getsockopt and it > >> revealed that getting a peer credential of a socket on FreeBSD > >> didn't work. Libvirtd hits the error: > >> error : virNetSocketGetUNIXIdentity:1198 : Failed to get valid > >> client socket identity groups > >> > >> SOL_SOCKET (0xffff) was used as a level of getsockopt for > >> LOCAL_PEERCRED, however, it was wrong. 0 is correct as well as > >> Mac OS X. > >> > >> So for LOCAL_PEERCRED our options are SOL_LOCAL (if defined) or > >> 0 on Mac OS X and FreeBSD. According to the fact, the patch > >> simplifies the code by removing ifdef __APPLE__. > >> > >> I tested the patch on FreeBSD 8.4, 9.2 and 10.0-BETA1. > >> > >> Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com&gt; > >> --- > >> src/rpc/virnetsocket.c | 21 ++++++++++----------- > >> 1 file changed, 10 insertions(+), 11 deletions(-) > >> > >> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c > >> index 3eb5708..04bf25a 100644 > >> --- a/src/rpc/virnetsocket.c > >> +++ b/src/rpc/virnetsocket.c > >> @@ -1152,18 +1152,17 @@ cleanup: > >> /* VIR_SOL_PEERCRED - the value needed to let getsockopt() work with > >> * LOCAL_PEERCRED > >> */ > >> -# ifdef __APPLE__ > >> -# ifdef SOL_LOCAL > >> -# define VIR_SOL_PEERCRED SOL_LOCAL > >> -# else > >> -/* Prior to Mac OS X 10.7, SOL_LOCAL was not defined and users were > >> - * expected to supply 0 as the second value for getsockopt() when using > >> - * LOCAL_PEERCRED > >> - */ > >> -# define VIR_SOL_PEERCRED 0 > >> -# endif > >> + > >> +/* Mac OS X 10.8 provides SOL_LOCAL for LOCAL_PEERCRED */ > >> +# ifdef SOL_LOCAL > >> +# define VIR_SOL_PEERCRED SOL_LOCAL > >> # else > >> -# define VIR_SOL_PEERCRED SOL_SOCKET > >> +/* FreeBSD and Mac OS X prior to 10.7, SOL_LOCAL is not defined and > >> + * users are expected to supply 0 as the second value for getsockopt() > >> + * when using LOCAL_PEERCRED. NB SOL_SOCKET cannot be used instead > >> + * of SOL_LOCAL > >> + */ > >> +# define VIR_SOL_PEERCRED 0 > >> # endif > >> > >> int virNetSocketGetUNIXIdentity(virNetSocketPtr sock, > >> -- > >> 1.8.4 > >> > > Confirmed this through a visual code inspection of the FreeBSD 9.2 > kernel. I also tested the patch through make check on FreeBSD 9.2 and > Linux (though make check does not exercise the issue at hand). I > believe its reasonable to push this for 1.1.4 so I'll go ahead and > push it now. ACK, go for it.

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