Bug#102437: dealloc analysis for each server function, more bugs!

[Marcus Brinkmann]

On Wed, Jul 11, 2001 at 12:18:32AM +0200, Marcus Brinkmann wrote:
> For the auth server, I will send a patch for review soon.

Here it comes.  I am not too happy about the macro magic, but it works out
ok.  The code for S_auth_getids is completely reduplicated in
S_auth_server_authenticate.  Do you want me to simply call S_auth_getids
instead?

Problem the patch wants to solve: Returned buffers are not page aligned if
they don't fit into the provided buffers, because they are malloc()'ed.

Way to fix it: If buffer is not big enough, allocate new buffer with mmap.
For this to work, all parameters must be "dealloc" for MiG (including the in
paramters of the reply stub).

Thanks,
Marcus

diff -ru hurd/auth/ChangeLog hurd-work/auth/ChangeLog
--- hurd/auth/ChangeLog Mon Feb 12 22:47:07 2001
+++ hurd-work/auth/ChangeLog    Wed Jul 11 21:18:00 2001
@@ -1,3 +1,22 @@
+2001-07-11  Marcus Brinkmann  <marcus@gnu.org>
+
+       * auth.c : Include <sys/mman.h>.
+       (idvec_copyout): Change return type from void to int.
+       Return buffer is allocated if not big enough.  Clean up
+       calculation of array size.  Return an error on failure and 0 on
+       success.
+       (C): Only call idvec_copyout if no error occured so far.
+       (OUTIDS): Change syntax to accomodate changes to definition of C.
+       (D, SAVEIDS): New macros to define local variables EUIDS_SAVED,
+       EGIDS_SAVED, AUIDS_SAVED, AGIDS_SAVED and store the respective
+       arguments in them.
+       (F, FREEIDS): New macros to free allocated buffers, to be used
+       on errors in conjunction with SAVEIDS.
+       (S_auth_getids): Use SAVEIDS and FREEIDS.
+       New variable err.
+       (S_auth_server_authenticate): Before calling the reply stub, copy
+       out the ids just as in S_auth_getids.
+
 2001-02-12  Marcus Brinkmann  <marcus@gnu.org>
 
        * auth.c (main): New variable ARGP defining a doc string.
diff -ru hurd/auth/auth.c hurd-work/auth/auth.c
--- hurd/auth/auth.c    Mon Feb 12 22:39:42 2001
+++ hurd-work/auth/auth.c       Wed Jul 11 21:16:51 2001
@@ -27,6 +27,7 @@
 #include <hurd/ports.h>
 #include <hurd/ihash.h>
 #include <idvec.h>
+#include <sys/mman.h>
 #include <assert.h>
 #include <argp.h>
 #include <error.h>
@@ -82,18 +83,30 @@
 
 /* id management.  */
 
-inline void idvec_copyout (struct idvec *idvec, uid_t **ids, uid_t *nids)
+inline int idvec_copyout (struct idvec *idvec, uid_t **ids, uid_t *nids)
 {
   if (idvec->num > *nids)
-    *ids = idvec->ids;
-  else
-    memcpy (*ids, idvec->ids, idvec->num * sizeof *ids);
+    {
+      *ids = mmap (0, idvec->num * sizeof (**ids), PROT_READ|PROT_WRITE,
+                  MAP_ANON, 0, 0);
+      if (*ids == (uid_t *) -1)
+       return errno;
+    }
+  memcpy (*ids, idvec->ids, idvec->num * sizeof (**ids));
   *nids = idvec->num;
+
+  return 0;
 }
 
-#define C(auth, ids)   idvec_copyout (&auth->ids, ids, n##ids)
-#define OUTIDS(auth)   (C (auth, euids), C (auth, egids), \
-                        C (auth, auids), C (auth, agids))
+#define C(auth, ids)   if (! err) \
+                         err = idvec_copyout (&auth->ids, ids, n##ids)
+#define OUTIDS(auth)   { C (auth, euids); C (auth, egids); \
+                         C (auth, auids); C (auth, agids); }
+#define D(ids) *ids##_saved = *ids
+#define SAVEIDS        uid_t D (euids), D (egids), D (auids), D (agids)
+#define F(ids) if (*ids != ids##_saved) \
+                 munmap (*ids, *n##ids * sizeof (uid_t))
+#define FREEIDS { F (euids); F (egids); F (auids); F (agids); }
 
 /* Implement auth_getids as described in <hurd/auth.defs>. */
 kern_return_t
@@ -107,12 +120,17 @@
               uid_t **agids,
               u_int *nagids)
 {
+  int err = 0;
+  SAVEIDS;
+
   if (! auth)
     return EOPNOTSUPP;
-
+  
   OUTIDS (auth);
-
-  return 0;
+  if (err)
+    FREEIDS;
+      
+  return err;
 }
 
 /* Implement auth_makeauth as described in <hurd/auth.defs>. */
@@ -421,6 +439,18 @@
   /* Extract the ids.  We must use a separate reply stub so
      we can deref the user auth handle after the reply uses its
      contents.  */
+  {
+    int err = 0;
+    SAVEIDS;
+
+    OUTIDS (user);
+
+    if (err)
+      {
+       FREEIDS;
+       return err;
+      }
+  }
   auth_server_authenticate_reply (reply, reply_type, 0,
                                  user->euids.ids, user->euids.num,
                                  user->auids.ids, user->auids.num,
diff -ru hurd/hurd/ChangeLog hurd-work/hurd/ChangeLog
--- hurd/hurd/ChangeLog Wed Jul 11 00:15:12 2001
+++ hurd-work/hurd/ChangeLog    Wed Jul 11 19:31:47 2001
@@ -1,5 +1,14 @@
 2001-07-11  Marcus Brinkmann  <marcus@gnu.org>
 
+       * auth_reply.defs (simpleroutine auth_server_authentcate_reply):
+       Add dealloc to in paramters (GEN_UIDS, AUX_UIDS, GEN_GIDS, AUX_GIDS).
+
+       * auth.defs (routine auth_getids): Add dealloc to all out parameters
+       (EUIDS, AUIDS, EGIDS, AGIDS).
+       (routine auth_server_authenticate): Likewise.
+
+2001-07-11  Marcus Brinkmann  <marcus@gnu.org>
+
        * fs.defs (routine file_get_storage_info): Add dealloc to all out
        parameters (PORTS, INTS, OFFSETS, DATA).
        (routine file_get_fs_options): Add dealloc to out parameter OPTIONS.
diff -ru hurd/hurd/auth.defs hurd-work/hurd/auth.defs
--- hurd/hurd/auth.defs Fri May  3 22:56:21 1996
+++ hurd-work/hurd/auth.defs    Wed Jul 11 19:28:18 2001
@@ -1,5 +1,5 @@
 /* Definitions for the authentication server
-   Copyright (C) 1991, 1992, 1993, 1994, 1996 Free Software Foundation
+   Copyright (C) 1991, 1992, 1993, 1994, 1996, 2001 Free Software Foundation
 
 This file is part of the GNU Hurd.
 
@@ -38,10 +38,10 @@
 /* Given an authentication handle, return the identification. */
 routine auth_getids (
        handle: auth_t;
-       out euids: idarray_t;
-       out auids: idarray_t;
-       out egids: idarray_t;
-       out agids: idarray_t);
+       out euids: idarray_t, dealloc;
+       out auids: idarray_t, dealloc;
+       out egids: idarray_t, dealloc;
+       out agids: idarray_t, dealloc);
 
 /* Create a new authentication handle.  */
 routine auth_makeauth (
@@ -72,9 +72,7 @@
        sreplyport reply: mach_port_poly_t;
        rendezvous: mach_port_send_t;
        newport: mach_port_poly_t;
-       out euids: idarray_t;
-       out auids: idarray_t;
-       out egids: idarray_t;
-       out agids: idarray_t);
-
-
+       out euids: idarray_t, dealloc;
+       out auids: idarray_t, dealloc;
+       out egids: idarray_t, dealloc;
+       out agids: idarray_t, dealloc);
diff -ru hurd/hurd/auth_reply.defs hurd-work/hurd/auth_reply.defs
--- hurd/hurd/auth_reply.defs   Mon Jan 24 23:38:38 1994
+++ hurd-work/hurd/auth_reply.defs      Wed Jul 11 19:31:36 2001
@@ -1,5 +1,5 @@
 /* Reply-only side of auth interface
-   Copyright (C) 1991, 1993, 1994 Free Software Foundation
+   Copyright (C) 1991, 1993, 1994, 2001 Free Software Foundation
 
 This file is part of the GNU Hurd.
 
@@ -37,8 +37,7 @@
 simpleroutine auth_server_authenticate_reply (
        reply_port: reply_port_t;
        in return_code: kern_return_t;
-       in gen_uids: idarray_t;
-       in aux_uids: idarray_t;
-       in gen_gids: idarray_t;
-       in aux_gids: idarray_t);
-       
+       in gen_uids: idarray_t, dealloc;
+       in aux_uids: idarray_t, dealloc;
+       in gen_gids: idarray_t, dealloc;
+       in aux_gids: idarray_t, dealloc);

-- 
`Rhubarb is no Egyptian god.' Debian http://www.debian.org brinkmd@debian.org
Marcus Brinkmann              GNU    http://www.gnu.org    marcus@gnu.org
Marcus.Brinkmann@ruhr-uni-bochum.de
http://www.marcus-brinkmann.de