memory corruption from mlock() changes

[Gerald (Jerry) Carter]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Guenther,

Gerald (Jerry) Carter wrote:
> Please try this patch that converts the talloc()'d memory
> to a malloc() before trying to mlock() it.  This clears
> up the crash and hangs on my Ubuntu systems.  Should be fine
> on my Solaris box as well but I've yet to test that.
> 
> If this works we could also convert to memalign as
> James suggested but would need to add configure tests
> I assume.  This is a simpler change for now.

New patch using memalign (or posix_memalign() depending
on what is available).  I tested my local tree on Linux
and Solaris.  Haven't tested this port to SAMBA_3_0 but
hopefully I didn't do anything stupid in the merge.




cheers, jerry
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.4 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF3yBcIR7qMdg1EfYRAg//AJoCL9QbAS+O8Zm7pnuSVaUjA3qyCQCgmW+J
2laBJnMggrGJ1twX32ssUgo=
=y8oP
-----END PGP SIGNATURE-----
-------------- next part --------------
Index: nsswitch/winbindd_cred_cache.c
===================================================================
--- nsswitch/winbindd_cred_cache.c	(revision 21520)
+++ nsswitch/winbindd_cred_cache.c	(working copy)
@@ -482,29 +482,21 @@
 		memcredp->len += strlen(pass)+1;
 	}
 
-	/* On non-linux platforms, mlock()'d memory must be aligned on
-	   a page boundary so allocate a bit more so we can offset
-	   enough */
+	/* On non-linux platforms, mlock()'d memory must be aligned */
 
-	memcredp->len += psize;
-			  
-	memcredp->buffer = (unsigned char*)TALLOC_ZERO(memcredp, memcredp->len);
-				  
-	if (!memcredp->buffer) {
+	memcredp->nt_hash = SMB_MEMALIGN_ARRAY(unsigned char*, psize, 
+					       memcredp->len);
+	if (!memcredp->nt_hash) {
 		return NT_STATUS_NO_MEMORY;
 	}
-				  
+	memset( memcredp->nt_hash, 0x0, memcredp->len );
 
-	/* point the nt_hash at the page boundary in the buffer */
-
-	memcredp->nt_hash = memcredp->buffer +
-		(psize - ((uint32)memcredp->buffer % psize));
 	memcredp->lm_hash = memcredp->nt_hash + NT_HASH_LEN;
 
 #ifdef DEBUG_PASSWORD
 	DEBUG(10,("mlocking memory: %p\n", memcredp->nt_hash));
 #endif		
-	if ((mlock(memcredp->nt_hash, memcredp->len-psize)) == -1) {
+	if ((mlock(memcredp->nt_hash, memcredp->len)) == -1) {
 		DEBUG(0,("failed to mlock memory: %s (%d)\n", 
 			strerror(errno), errno));
 		return map_nt_error_from_unix(errno);
@@ -536,15 +528,13 @@
 #if !defined(HAVE_MUNLOCK)
 	return NT_STATUS_OK;
 #else
-	int psize = getpagesize();	
-
-	if (munlock(memcredp->buffer, memcredp->len - psize) == -1) {
+	if (munlock(memcredp->nt_hash, memcredp->len) == -1) {
 		DEBUG(0,("failed to munlock memory: %s (%d)\n", 
 			strerror(errno), errno));
 		return map_nt_error_from_unix(errno);
 	}
-	memset(memcredp->buffer, '\0', memcredp->len);
-	TALLOC_FREE(memcredp->buffer);
+	memset(memcredp->nt_hash, '\0', memcredp->len);
+	SAFE_FREE(memcredp->nt_hash);
 	memcredp->nt_hash = NULL;
 	memcredp->lm_hash = NULL;
 	memcredp->pass = NULL;
Index: nsswitch/winbindd_nss.h
===================================================================
--- nsswitch/winbindd_nss.h	(revision 21520)
+++ nsswitch/winbindd_nss.h	(working copy)
@@ -469,8 +469,6 @@
 	uid_t uid;
 	int ref_count;
 	size_t len;
-	unsigned char *buffer;  /* buffer block containing the
-				   following 3 */
 	unsigned char *nt_hash; /* Base pointer for the following 2 */
 	unsigned char *lm_hash;
 	char *pass;
Index: lib/util.c
===================================================================
--- lib/util.c	(revision 21520)
+++ lib/util.c	(working copy)
@@ -913,6 +913,17 @@
 }
 
 /****************************************************************************
+ Internal malloc wrapper. Externally visible.
+****************************************************************************/
+
+void *memalign_(size_t align, size_t size)
+{
+#undef memalign
+	return memalign(align, size);
+#define memalign(align, s) __ERROR_DONT_USE_MEMALIGN_DIRECTLY
+}
+
+/****************************************************************************
  Internal calloc wrapper. Not externally visible.
 ****************************************************************************/
 
@@ -954,6 +965,23 @@
 }
 
 /****************************************************************************
+ Type-safe memalign
+****************************************************************************/
+
+void *memalign_array(size_t el_size, size_t align, unsigned int count)
+{
+	if (count >= MAX_ALLOC_SIZE/el_size) {
+		return NULL;
+	}
+
+#if defined(PARANOID_MALLOC_CHECKER)
+	return memalign_(align, el_size*count);
+#else
+	return sys_memalign(align, el_size*count);
+#endif
+}
+
+/****************************************************************************
  Type-safe calloc.
 ****************************************************************************/
 
Index: lib/system.c
===================================================================
--- lib/system.c	(revision 21520)
+++ lib/system.c	(working copy)
@@ -44,6 +44,27 @@
 
 
 /*******************************************************************
+ A wrapper for memalign
+********************************************************************/
+
+void* sys_memalign( size_t align, size_t size )
+{
+#if defined(HAVE_MEMALIGN)
+	return memalign( align, size );
+#elif defined(HAVE_POSIX_MEMALIGN)
+	char *p = NULL;
+	int ret = posix_memalign( &p, align, size );
+	if ( ret == 0 )
+		return p;
+		
+	return NULL;
+#else
+	DEBUG(0,("memalign functionalaity not available on this platform!\n"));
+	return NULL;
+#endif
+}
+
+/*******************************************************************
  A wrapper for usleep in case we don't have one.
 ********************************************************************/
 
Index: include/smb_macros.h
===================================================================
--- include/smb_macros.h	(revision 21520)
+++ include/smb_macros.h	(working copy)
@@ -276,6 +276,7 @@
 *****************************************************************************/
 
 #define SMB_MALLOC_ARRAY(type,count) (type *)malloc_array(sizeof(type),(count))
+#define SMB_MEMALIGN_ARRAY(type,align,count) (type *)memalign_array(sizeof(type),align,(count))
 #define SMB_REALLOC(p,s) Realloc((p),(s),True)	/* Always frees p on error or s == 0 */
 #define SMB_REALLOC_KEEP_OLD_ON_ERROR(p,s) Realloc((p),(s),False) /* Never frees p on error or s == 0 */
 #define SMB_REALLOC_ARRAY(p,type,count) (type *)realloc_array((p),sizeof(type),(count),True) /* Always frees p on error or s == 0 */
Index: configure.in
===================================================================
--- configure.in	(revision 21520)
+++ configure.in	(working copy)
@@ -1240,6 +1240,7 @@
 AC_CHECK_FUNCS(setlocale nl_langinfo)
 AC_CHECK_FUNCS(nanosleep)
 AC_CHECK_FUNCS(mlock munlock mlockall munlockall)
+AC_CHECK_FUNCS(memalign posix_memalign)
 AC_CHECK_HEADERS(sys/mman.h)
 # setbuffer, shmget, shm_open are needed for smbtorture
 AC_CHECK_FUNCS(setbuffer shmget shm_open)