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

[Ryota Ozaki]

On Mon, Nov 4, 2013 at 10:37 AM, Daniel P. Berrange <berrange at redhat.com> wrote:
> 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 at gmail.com> 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 at gmail.com> 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 at gmail.com>
>> >> ---
>> >>  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.

Thanks!

  ozaki-r

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