bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Marcus Brinkmann
Subject: Bug#102437: dealloc analysis for each server function, more bugs!
Date: Thu, 12 Jul 2001 21:37:51 +0200
User-agent: Mutt/1.3.18i

On Thu, Jul 12, 2001 at 09:06:12PM +0200, Marcus Brinkmann wrote:
> On Wed, Jul 11, 2001 at 04:49:17PM -0400, Roland McGrath wrote:
> > That's fine by me if it's tested.  BTW add -p to your diff switches.
> 
> Actually, I goofed up.  The arguments to the reply stub have not been
> changed by my patch.  In fact, as we now copy the user data all the time, a
> seperate reply stub is not necessary anymore!  There is also a bug
> somewhere.

The bug was actually what I said above.  The following version is tested and
seems to work fine.

Note that it got much simpler, because the extra reply stub is not needed. 
I left the change to auth_reply.defs in anyway.  However, nothing in the
Hurd uses the stubs in auth_reply.defs anymore.

Thanks,
Marcus

hurd/
2001-07-11  Marcus Brinkmann  <address@hidden>

        * 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.

auth/
2001-07-11  Marcus Brinkmann  <address@hidden>

        * 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): Use SAVEIDS, make ERR a local variable
        of the whole function.  Instead calling the reply stub, copy out the
        ids just as in S_auth_getids.  Return ERR.


--- hurd/hurd/auth.defs Fri May  3 22:56:21 1996
+++ /mnt/marcus/gnu/hurd/hurd/hurd-20010610/hurd/auth.defs      Wed Jul 11 
20:44:38 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);

--- hurd/hurd/auth_reply.defs   Mon Jan 24 23:38:38 1994
+++ /mnt/marcus/gnu/hurd/hurd/hurd-20010610/hurd/auth_reply.defs        Wed Jul 
11 20:44:38 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);

diff -pru hurd/auth/auth.c /mnt/marcus/gnu/hurd/hurd/hurd-20010610/auth/auth.c
--- hurd/auth/auth.c    Mon Feb 12 22:39:42 2001
+++ /mnt/marcus/gnu/hurd/hurd/hurd-20010610/auth/auth.c Thu Jul 12 21:09:31 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 @@ auth_port_to_handle (auth_t auth)
 
 /* 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 @@ S_auth_getids (struct authhandle *auth,
               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>. */
@@ -358,8 +376,10 @@ S_auth_server_authenticate (struct authh
                            uid_t **agids,
                            u_int *nagids)
 {
+  error_t err = 0;
   struct pending *u;
   struct authhandle *user;
+  SAVEIDS;
 
   if (! serverauth)
     return EOPNOTSUPP;
@@ -392,7 +412,6 @@ S_auth_server_authenticate (struct authh
       /* No pending user RPC for this port.
         Create a pending server RPC record.  */
       struct pending s;
-      error_t err;
 
       err = ihash_add (pending_servers, rendezvous, &s, &s.locp);
       if (! err)
@@ -418,17 +437,13 @@ S_auth_server_authenticate (struct authh
       user = s.user;
     }
 
-  /* Extract the ids.  We must use a separate reply stub so
-     we can deref the user auth handle after the reply uses its
-     contents.  */
-  auth_server_authenticate_reply (reply, reply_type, 0,
-                                 user->euids.ids, user->euids.num,
-                                 user->auids.ids, user->auids.num,
-                                 user->egids.ids, user->egids.num,
-                                 user->agids.ids, user->agids.num);
+  OUTIDS (user);
+  if (err)
+    FREEIDS;
+
   ports_port_deref (user);
   mach_port_deallocate (mach_task_self (), rendezvous);
-  return MIG_NO_REPLY;
+  return err;
 }
 
 

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]