[libvirt] PATCH: better error checking for LOCAL_PEERCRED

[Daniel P. Berrange]

On Thu, Oct 17, 2013 at 06:29:14AM -0600, Eric Blake wrote:
> On 10/16/2013 10:08 AM, Brian Candler wrote:
> > On 15/10/2013 12:16, Daniel P. Berrange wrote:
> >> 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.
> > Sorry about that. Updated patch uploaded to
> > https://bugzilla.redhat.com/show_bug.cgi?id=1019929 where Thunderbird
> > can't mangle it.
> 
> On the other hand, making someone chase a URL is less convenient than
> attaching the patch (ideally, sending inline the way 'git send-email'
> does things is preferred, but since 'git am' can also handle
> attachments, it's still easier on the maintainer to send through the
> list).  That said, I went ahead and did the work for you this time around.
> 
> Here's what I pushed:
> 
> From aa0f09929d02ccdbf3ca9502a1fd39d90db0c690 Mon Sep 17 00:00:00 2001
> From: Brian Candler <b.candler at pobox.com>
> Date: Thu, 17 Oct 2013 06:21:57 -0600
> Subject: [PATCH] better error checking for LOCAL_PEERCRED
> 
> This patch improves the error checking in the LOCAL_PEERCRED version
> of virNetSocketGetUNIXIdentity, used by FreeBSD and Mac OSX.
> 
> 1. The error return paths now correctly unlock the socket. This is
> implemented in exactly the same way as the SO_PEERCRED version,
> using "goto cleanup"
> 
> 2. cr.cr_ngroups is initialised to -1, and cr.cr_ngroups is checked
> for negative and overlarge values.
> 
> This means that if the getsockopt() call returns success but doesn't
> actually update the xucred structure, this is now caught. This
> happened previously when getsockopt was called with SOL_SOCKET
> instead of SOL_LOCAL, prior to commit 5a468b3, and resulted in
> random uids being accepted.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/rpc/virnetsocket.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index e8cdfa6..2e50f8c 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -1174,25 +1174,27 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr
> sock,
>  {
>      struct xucred cr;
>      socklen_t cr_len = sizeof(cr);
> +    int ret = -1;
> +
>      virObjectLock(sock);
> 
> +    cr.cr_ngroups = -1;
>      if (getsockopt(sock->fd, VIR_SOL_PEERCRED, LOCAL_PEERCRED, &cr,
> &cr_len) < 0) {

Hehe, your email client is mangling line breaks too Eric :-P

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