Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 29 Mar 2012 03:13:43 +0000 (UTC)
From:      Juli Mallett <jmallett@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r233646 - head/lib/libgeom
Message-ID:  <201203290313.q2T3Dh8p073473@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jmallett
Date: Thu Mar 29 03:13:43 2012
New Revision: 233646
URL: http://svn.freebsd.org/changeset/base/233646

Log:
  Fix 32-bit libgeom consumers run on 64-bit kernels with COMPAT_FREEBSD32.
  
  Kernel pointer values are used as opaque unique identifiers, which are then
  used to reconstruct references between various providers, classes, etc., inside
  libgeom from the source XML.  Unfortunately, they're converted to pointer-width
  integers (in the form of pointers) to do this, and 32-bit userland pointers
  cannot hold sensible representations (however opaque) of 64-bit kernel pointers
  on all systems.
  
  In the case where the leading bits are zero and 32 distinct bits of pointer can
  be identified, this will happen to work.  On systems where the upper 32-bits of
  kernel pointers are non-zero and the same for all kernel pointers, this will
  result in double frees and all kinds of bizarre crashes and linkage between
  objects inside libgeom.
  
  To mitigate this problem, treat the opaque identifiers in the XML as C strings
  instead, and internalize them to give unique and consistent per-object pointer
  values in userland for each identifier in the XML.  This allows us to keep the
  libgeom logic the same with only minor changes to initial setup and parsing.
  
  It might be more sensible for speed reasons to treat the identifiers as numbers
  of a large size (uintmax_t, say) rather than strings, but strings seem fine for
  now.
  
  (As an added side-effect, this makes it slightly easier to identify unresolved
   references, but nothing has been added to inform the user of those.)

Modified:
  head/lib/libgeom/geom_xml2tree.c
  head/lib/libgeom/libgeom.h

Modified: head/lib/libgeom/geom_xml2tree.c
==============================================================================
--- head/lib/libgeom/geom_xml2tree.c	Thu Mar 29 03:04:59 2012	(r233645)
+++ head/lib/libgeom/geom_xml2tree.c	Thu Mar 29 03:13:43 2012	(r233646)
@@ -55,9 +55,56 @@ struct mystate {
 	int			level;
 	struct sbuf		*sbuf[20];
 	struct gconf		*config;
-	int			nident;
+	unsigned		nident;
 };
 
+static void *
+internalize_ident(struct mystate *mt, const char *element, const char *str)
+{
+	struct gident *gip;
+	unsigned i;
+
+	if (mt->nident != 0 && mt->mesh->lg_ident == NULL) {
+		warn("Cannot continue due to previous memory exhaustion.");
+		return (NULL);
+	}
+
+	for (i = 0; i < mt->nident; i++) {
+		if (strcmp(mt->mesh->lg_ident[i].lg_id, str) != 0)
+			continue;
+		return ((void *)(uintptr_t)(i + 1));
+	}
+
+	i = mt->nident;
+	mt->nident++;
+	mt->mesh->lg_ident = reallocf(mt->mesh->lg_ident, (mt->nident + 1) * sizeof mt->mesh->lg_ident[0]);
+	if (mt->mesh->lg_ident == NULL) {
+		warn("Cannot allocate memory during processing of '%s' "
+		    "element for identifier '%s'", element, str);
+		return (NULL);
+	}
+
+	gip = &mt->mesh->lg_ident[i];
+	gip->lg_id = strdup(str);
+	if (gip->lg_id == NULL) {
+		free(mt->mesh->lg_ident);
+		mt->mesh->lg_ident = NULL;
+		warn("Cannot allocate memory during processing of '%s' "
+		    "element for identifier '%s'", element, str);
+		return (NULL);
+	}
+	gip->lg_ptr = NULL;
+	gip->lg_what = ISUNRESOLVED;
+
+	/* Terminator entry.  */
+	gip = &mt->mesh->lg_ident[i + 1];
+	gip->lg_id = NULL;
+	gip->lg_ptr = NULL;
+	gip->lg_what = ISUNRESOLVED;
+
+	return ((void *)(uintptr_t)(i + 1));
+}
+
 static void
 StartElement(void *userData, const char *name, const char **attr)
 {
@@ -73,10 +120,9 @@ StartElement(void *userData, const char 
 	ref = NULL;
 	for (i = 0; attr[i] != NULL; i += 2) {
 		if (!strcmp(attr[i], "id")) {
-			id = (void *)strtoul(attr[i + 1], NULL, 0);
-			mt->nident++;
+			id = internalize_ident(mt, name, attr[i + 1]);
 		} else if (!strcmp(attr[i], "ref")) {
-			ref = (void *)strtoul(attr[i + 1], NULL, 0);
+			ref = internalize_ident(mt, name, attr[i + 1]);
 		} else
 			printf("%*.*s[%s = %s]\n",
 			    mt->level + 1, mt->level + 1, "",
@@ -317,11 +363,16 @@ CharData(void *userData , const XML_Char
 struct gident *
 geom_lookupid(struct gmesh *gmp, const void *id)
 {
-	struct gident *gip;
+	unsigned i;
 
-	for (gip = gmp->lg_ident; gip->lg_id != NULL; gip++)
-		if (gip->lg_id == id)
-			return (gip);
+	if (gmp->lg_ident == NULL)
+		return (NULL);
+
+	for (i = 0; gmp->lg_ident[i].lg_id != NULL; i++) {
+		if (i + 1 != (unsigned)(uintptr_t)id)
+			continue;
+		return (&gmp->lg_ident[i]);
+	}
 	return (NULL);
 }
 
@@ -334,6 +385,7 @@ geom_xml2tree(struct gmesh *gmp, char *p
 	struct ggeom *ge;
 	struct gprovider *pr;
 	struct gconsumer *co;
+	struct gident *gip;
 	int i;
 
 	memset(gmp, 0, sizeof *gmp);
@@ -356,33 +408,30 @@ geom_xml2tree(struct gmesh *gmp, char *p
 		free(mt);
 		return (-1);
 	}
-	gmp->lg_ident = calloc(sizeof *gmp->lg_ident, mt->nident + 1);
-	free(mt);
-	if (gmp->lg_ident == NULL)
+	if (gmp->lg_ident == NULL && mt->nident != 0) {
+		free(mt);
 		return (ENOMEM);
-	i = 0;
+	}
+	free(mt);
 	/* Collect all identifiers */
 	LIST_FOREACH(cl, &gmp->lg_class, lg_class) {
-		gmp->lg_ident[i].lg_id = cl->lg_id;
-		gmp->lg_ident[i].lg_ptr = cl;
-		gmp->lg_ident[i].lg_what = ISCLASS;
-		i++;
+		gip = geom_lookupid(gmp, cl->lg_id);
+		gip->lg_ptr = cl;
+		gip->lg_what = ISCLASS;
+
 		LIST_FOREACH(ge, &cl->lg_geom, lg_geom) {
-			gmp->lg_ident[i].lg_id = ge->lg_id;
-			gmp->lg_ident[i].lg_ptr = ge;
-			gmp->lg_ident[i].lg_what = ISGEOM;
-			i++;
+			gip = geom_lookupid(gmp, ge->lg_id);
+			gip->lg_ptr = ge;
+			gip->lg_what = ISGEOM;
 			LIST_FOREACH(pr, &ge->lg_provider, lg_provider) {
-				gmp->lg_ident[i].lg_id = pr->lg_id;
-				gmp->lg_ident[i].lg_ptr = pr;
-				gmp->lg_ident[i].lg_what = ISPROVIDER;
-				i++;
+				gip = geom_lookupid(gmp, pr->lg_id);
+				gip->lg_ptr = pr;
+				gip->lg_what = ISPROVIDER;
 			}
 			LIST_FOREACH(co, &ge->lg_consumer, lg_consumer) {
-				gmp->lg_ident[i].lg_id = co->lg_id;
-				gmp->lg_ident[i].lg_ptr = co;
-				gmp->lg_ident[i].lg_what = ISCONSUMER;
-				i++;
+				gip = geom_lookupid(gmp, co->lg_id);
+				gip->lg_ptr = co;
+				gip->lg_what = ISCONSUMER;
 			}
 		}
 	}
@@ -449,7 +498,10 @@ geom_deletetree(struct gmesh *gmp)
 	struct ggeom *ge;
 	struct gprovider *pr;
 	struct gconsumer *co;
+	unsigned i;
 
+	for (i = 0; gmp->lg_ident[i].lg_id != NULL; i++)
+		free(gmp->lg_ident[i].lg_id);
 	free(gmp->lg_ident);
 	gmp->lg_ident = NULL;
 	for (;;) {

Modified: head/lib/libgeom/libgeom.h
==============================================================================
--- head/lib/libgeom/libgeom.h	Thu Mar 29 03:04:59 2012	(r233645)
+++ head/lib/libgeom/libgeom.h	Thu Mar 29 03:13:43 2012	(r233646)
@@ -71,7 +71,8 @@ struct gident {
 	enum {	ISCLASS,
 		ISGEOM,
 		ISPROVIDER,
-		ISCONSUMER }	lg_what;
+		ISCONSUMER,
+		ISUNRESOLVED }	lg_what;
 };
 
 struct gmesh {

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203290313.q2T3Dh8p073473>