Re: [libvirt] [PATCH]: don't harcode buffer for getgrnam_r

[Guido Günther <agx@xxxxxxxxxxx>]

On Thu, Apr 16, 2009 at 09:56:27AM +0200, Daniel Veillard wrote:
> On Thu, Apr 16, 2009 at 09:19:38AM +0200, Guido Günther wrote:
> > Hi,
> > determines the maximum needed buffersize for getgrnam_r using sysconf
> > instead of hardcoding it to 1024 and increases the buffer on ERANGE.
> > The latter is needed since sysconf is allowed to return -1. Furthermore
> > some glibc versions seem to return a too small buffer on amd64
> > (http://bugs.debian.org/520744). O.k. to apply?
> 
>   It looks a bit weird, using sysconf but 1/ allowing it to fail so
> doing the 2/ 1024 value and loop on ERANGE , but well if I understand
> correctly taht's forced by some glibc broken behaviour.
Yes, sysconf is allowed to return -1 here.

>   My take is that the *= 2 size loop should be bounded to avoid eating
> all memory on some intermediate not size related error. Can we really
glibc shouldn't return ERANGE then, but better safe than sorry. I've
added that check in the attched patch.
Cheers,
 -- Guido

>From 6b19ee2f37e090c96d98da62f7268fb07da112e6 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx@xxxxxxxxxxx>
Date: Tue, 14 Apr 2009 18:08:16 +0200
Subject: [PATCH] increase buffer on ERANGE

---
 qemud/qemud.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/qemud/qemud.c b/qemud/qemud.c
index 4f04355..2a85a90 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -2529,6 +2529,7 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename)
     char *unix_sock_ro_perms = NULL;
     char *unix_sock_rw_perms = NULL;
     char *unix_sock_group = NULL;
+    char *buf = NULL;
 
 #if HAVE_POLKIT
     /* Change the default back to no auth for non-root */
@@ -2574,13 +2575,34 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename)
         if (getuid() != 0) {
             VIR_WARN0(_("Cannot set group when not running as root"));
         } else {
-            char buf[1024];
+            int ret;
             struct group grpdata, *grp;
-            if (getgrnam_r(unix_sock_group, &grpdata, buf, sizeof(buf), &grp) != 0 || !grp) {
+            size_t maxbuf = sysconf(_SC_GETGR_R_SIZE_MAX);
+
+            if (maxbuf == -1)
+                maxbuf = 1024;
+
+            if (VIR_ALLOC_N(buf, maxbuf) < 0) {
+                VIR_ERROR("%s", _("Failed to allocate memory for buffer"));
+                goto free_and_fail;
+            }
+
+            while ((ret = getgrnam_r(unix_sock_group, &grpdata,
+                                     buf, maxbuf,
+                                     &grp)) == ERANGE) {
+                    maxbuf *= 2;
+                    if (maxbuf > 65536 || VIR_REALLOC_N(buf, maxbuf) < 0) {
+                        VIR_ERROR("%s", _("Failed to reallocate enough memory for buffer"));
+                        goto free_and_fail;
+                    }
+            }
+
+            if (ret != 0 || !grp) {
                 VIR_ERROR(_("Failed to lookup group '%s'"), unix_sock_group);
                 goto free_and_fail;
             }
             unix_sock_gid = grp->gr_gid;
+            VIR_FREE (buf);
         }
         free (unix_sock_group);
         unix_sock_group = NULL;
@@ -2643,6 +2665,7 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename)
     free (unix_sock_ro_perms);
     free (unix_sock_rw_perms);
     free (unix_sock_group);
+    VIR_FREE (buf);
 
     /* Don't bother trying to free listen_addr, tcp_port, tls_port, key_file,
        cert_file, ca_file, or crl_file, since they are initialized to
-- 
1.6.2.1

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list