[OpenAFS] Re: vmalloc troubles with 1.4.2

chas williams - CONTRACTOR chas@cmf.nrl.navy.mil
Wed, 06 Dec 2006 08:11:53 -0500


In message <4575D1AB.70800@cox.net>,Joe Buehler writes:
>but an alternate patch was coded up by someone, but never tested at large,
>so it never made it to 1.4.2.

i suppose my suggested change was a little more drastic.  afs has
enough mutexs and linux already has storage pools via the slab
allocator.  i have tested this a bit locally (nothing like 40-50
users though).

this patch also removes a spurious memset() and fixed crdup() to copy
the rgid (instead of leaving it 0).  i guess that might be a security
flaw except that i dont think crdup() is used by any non-root code.


Index: osi_cred.c
===================================================================
--- osi_cred.c	(revision 5)
+++ osi_cred.c	(working copy)
@@ -20,49 +20,18 @@
 #include "afs/sysincludes.h"
 #include "afsincludes.h"
 
-/* Setup a pool for creds. Allocate several at a time. */
-#define CRED_ALLOC_STEP 29	/* at 140 bytes/cred = 4060 bytes. */
-
-
-static cred_t *cred_pool = NULL;
-int cred_allocs = 0;
-int ncreds_inuse = 0;
-
-/* Cred locking assumes current single threaded non-preemptive kernel.
- * Also assuming a fast path through both down and up if no waiters. Otherwise,
- * test if no creds in pool before grabbing lock in crfree().
- */
-#if defined(AFS_LINUX24_ENV)
-static DECLARE_MUTEX(linux_cred_pool_lock);
-#else
-static struct semaphore linux_cred_pool_lock = MUTEX;
-#endif
-#define CRED_LOCK() down(&linux_cred_pool_lock)
-#define CRED_UNLOCK() up(&linux_cred_pool_lock)
-
 cred_t *
 crget(void)
 {
     cred_t *tmp;
-    int i;
 
-    CRED_LOCK();
-    if (!cred_pool) {
-	cred_allocs++;
-	cred_pool = (cred_t *) osi_Alloc(CRED_ALLOC_STEP * sizeof(cred_t));
-	if (!cred_pool)
+#if !defined(GFP_NOFS)
+#define GFP_NOFS GFP_KERNEL
+#endif
+    tmp = kmalloc(sizeof(cred_t), GFP_NOFS);
+    if (!tmp)
 	    osi_Panic("crget: No more memory for creds!\n");
 
-	for (i = 0; i < CRED_ALLOC_STEP - 1; i++)
-	    cred_pool[i].cr_next = (cred_t *) &cred_pool[i + 1];
-	cred_pool[i].cr_next = NULL;
-    }
-    tmp = cred_pool;
-    cred_pool = (cred_t *) tmp->cr_next;
-    ncreds_inuse++;
-    CRED_UNLOCK();
-
-    memset(tmp, 0, sizeof(cred_t));
     tmp->cr_ref = 1;
     return tmp;
 }
@@ -75,14 +44,7 @@
 	return;
     }
 
-#if defined(AFS_LINUX26_ENV)
-    put_group_info(cr->cr_group_info);
-#endif
-    CRED_LOCK();
-    cr->cr_next = (cred_t *) cred_pool;
-    cred_pool = cr;
-    CRED_UNLOCK();
-    ncreds_inuse--;
+    kfree(cr);
 }
 
 
@@ -95,6 +57,7 @@
     tmp->cr_uid = cr->cr_uid;
     tmp->cr_ruid = cr->cr_ruid;
     tmp->cr_gid = cr->cr_gid;
+    tmp->cr_rgid = cr->cr_rgid;
 
 #if defined(AFS_LINUX26_ENV)
     get_group_info(cr->cr_group_info);
@@ -104,7 +67,6 @@
     tmp->cr_ngroups = cr->cr_ngroups;
 #endif
 
-    tmp->cr_ref = 1;
     return tmp;
 }