bin/83464: [ PATCH ] Unhandled malloc failures within libgeom
Dan Lukes
dan at obluda.cz
Fri Jul 15 16:40:23 GMT 2005
The following reply was made to PR bin/83464; it has been noted by GNATS.
From: Dan Lukes <dan at obluda.cz>
To: bug-followup at FreeBSD.org
Cc:
Subject: Re: bin/83464: [ PATCH ] Unhandled malloc failures within libgeom
Date: Fri, 15 Jul 2005 18:32:22 +0200
This is a multi-part message in MIME format.
--------------070408060806070200010105
Content-Type: text/plain; charset=ISO-8859-2; format=flowed
Content-Transfer-Encoding: 7bit
Well, so rewritten patch follows.
1. get_geomxml() is rewritten
2. The recent gctl_check_alloc() assign pointer to constant string when
dynamic allocation of error message failed - but gctl_free try to free
the req->error even if not allocated dynamically. Corrected.
3. realloc() within gctl_new_arg can cause memory leak. Changed to
reallocf()
4. ap->name=strdup(name) within gctl_ro_param() and gctl_rw_param() not
checked for failures. Corrected.
5. several malloc()/calloc()/strdup() used within StartElement() and
EndElement() not checked for failures. As those functions have no way to
return eror, I use the warn() then return. The XML parsing should fail
later due syntax error because element in question has not been processed.
WARNS can be raised to 6
Dan
--------------070408060806070200010105
Content-Type: text/plain;
name="patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="patch"
--- lib/libgeom/geom_getxml.c.ORIG Mon Feb 10 01:11:43 2003
+++ lib/libgeom/geom_getxml.c Fri Jul 15 17:22:18 2005
@@ -39,28 +39,21 @@
geom_getxml()
{
char *p;
- size_t l;
- int i;
+ size_t l = 0;
+ int mib[3];
+ size_t sizep = sizeof(mib)/sizeof(*mib);
- l = 1024 * 1024; /* Start big, realloc back */
- p = malloc(l);
- if (p) {
- i = sysctlbyname("kern.geom.confxml", p, &l, NULL, 0);
- if (i == 0) {
- p = realloc(p, strlen(p) + 1);
- return (p);
- }
+ if (sysctlnametomib("kern.geom.confxml", mib, &sizep) != 0)
+ return (NULL);
+ if (sysctl(mib, sizep, NULL, &l, NULL, 0) != 0)
+ return (NULL);
+ l+=4096;
+ if ((p = malloc(l)) == NULL)
+ return (NULL);
+ if (sysctl(mib, sizep, p, &l, NULL, 0) != 0) {
free(p);
- }
- l = 0;
- i = sysctlbyname("kern.geom.confxml", p, &l, NULL, 0);
- if (i != 0)
return (NULL);
- p = malloc(l + 4096);
- i = sysctlbyname("kern.geom.confxml", p, &l, NULL, 0);
- if (i == 0) {
- p = realloc(p, strlen(p) + 1);
- return (p);
}
- return (NULL);
+
+ return (reallocf(p, strlen(p) + 1));
}
--- lib/libgeom/geom_ctl.c.ORIG Mon Jun 2 20:54:50 2003
+++ lib/libgeom/geom_ctl.c Fri Jul 15 18:01:44 2005
@@ -45,6 +45,8 @@
#define GCTL_TABLE 1
#include <libgeom.h>
+char nomemmsg[] = "Could not allocate memory";
+
void
gctl_dump(struct gctl_req *req, FILE *f)
{
@@ -109,7 +111,7 @@
return;
gctl_set_error(req, "Could not allocate memory");
if (req->error == NULL)
- req->error = "Could not allocate memory";
+ req->error = nomemmsg;
}
/*
@@ -134,7 +136,7 @@
struct gctl_req_arg *ap;
req->narg++;
- req->arg = realloc(req->arg, sizeof *ap * req->narg);
+ req->arg = reallocf(req->arg, sizeof *ap * req->narg);
gctl_check_alloc(req, req->arg);
if (req->arg == NULL) {
req->narg = 0;
@@ -149,14 +151,20 @@
gctl_ro_param(struct gctl_req *req, const char *name, int len, const void* value)
{
struct gctl_req_arg *ap;
+ char *ap_name;
if (req == NULL || req->error != NULL)
return;
+ ap_name = strdup(name);
+ gctl_check_alloc(req, ap_name);
+ if (ap_name == NULL)
+ return;
ap = gctl_new_arg(req);
- if (ap == NULL)
+ if (ap == NULL) {
+ free(ap_name);
return;
- ap->name = strdup(name);
- gctl_check_alloc(req, ap->name);
+ }
+ ap->name = ap_name;
ap->nlen = strlen(ap->name) + 1;
ap->value = __DECONST(void *, value);
ap->flag = GCTL_PARAM_RD;
@@ -172,14 +180,20 @@
gctl_rw_param(struct gctl_req *req, const char *name, int len, void* value)
{
struct gctl_req_arg *ap;
+ char *ap_name;
if (req == NULL || req->error != NULL)
return;
+ ap_name = strdup(name);
+ gctl_check_alloc(req, ap_name);
+ if (ap_name == NULL)
+ return;
ap = gctl_new_arg(req);
- if (ap == NULL)
+ if (ap == NULL) {
+ free(ap_name);
return;
- ap->name = strdup(name);
- gctl_check_alloc(req, ap->name);
+ }
+ ap->name = ap_name;
ap->nlen = strlen(ap->name) + 1;
ap->value = value;
ap->flag = GCTL_PARAM_RW;
@@ -201,12 +215,11 @@
req->version = GCTL_VERSION;
req->lerror = BUFSIZ; /* XXX: arbitrary number */
- req->error = malloc(req->lerror);
+ req->error = calloc(1, req->lerror);
if (req->error == NULL) {
gctl_check_alloc(req, req->error);
return (req->error);
}
- memset(req->error, 0, req->lerror);
req->lerror--;
fd = open(_PATH_DEV PATH_GEOM_CTL, O_RDONLY);
if (fd < 0)
@@ -232,7 +245,7 @@
free(req->arg[i].name);
}
free(req->arg);
- if (req->error != NULL)
+ if (req->error != NULL && req->error != nomemmsg)
free(req->error);
free(req);
}
--- lib/libgeom/Makefile.ORIG Thu Jul 14 16:34:19 2005
+++ lib/libgeom/Makefile Fri Jul 15 17:44:37 2005
@@ -10,7 +10,7 @@
CFLAGS += -I${.CURDIR}
-WARNS?= 3
+WARNS?= 6
DPADD= ${LIBBSDXML} ${LIBSBUF}
LDADD= -lbsdxml -lsbuf
--- lib/libgeom/geom_xml2tree.c.ORIG Tue May 24 12:10:38 2005
+++ lib/libgeom/geom_xml2tree.c Fri Jul 15 18:13:14 2005
@@ -84,6 +84,10 @@
}
if (!strcmp(name, "class") && mt->class == NULL) {
mt->class = calloc(1, sizeof *mt->class);
+ if (mt->class == NULL) {
+ warn("Problem during processing of '%s' element", name);
+ return;
+ }
mt->class->lg_id = id;
LIST_INSERT_HEAD(&mt->mesh->lg_class, mt->class, lg_class);
LIST_INIT(&mt->class->lg_geom);
@@ -92,6 +96,10 @@
}
if (!strcmp(name, "geom") && mt->geom == NULL) {
mt->geom = calloc(1, sizeof *mt->geom);
+ if (mt->geom == NULL) {
+ warn("Problem during processing of '%s' element", name);
+ return;
+ }
mt->geom->lg_id = id;
LIST_INSERT_HEAD(&mt->class->lg_geom, mt->geom, lg_geom);
LIST_INIT(&mt->geom->lg_provider);
@@ -105,6 +113,10 @@
}
if (!strcmp(name, "consumer") && mt->consumer == NULL) {
mt->consumer = calloc(1, sizeof *mt->consumer);
+ if (mt->consumer == NULL) {
+ warn("Problem during processing of '%s' element", name);
+ return;
+ }
mt->consumer->lg_id = id;
LIST_INSERT_HEAD(&mt->geom->lg_consumer, mt->consumer,
lg_consumer);
@@ -121,6 +133,10 @@
}
if (!strcmp(name, "provider") && mt->provider == NULL) {
mt->provider = calloc(1, sizeof *mt->provider);
+ if (mt->provider == NULL) {
+ warn("Problem during processing of '%s' element", name);
+ return;
+ }
mt->provider->lg_id = id;
LIST_INSERT_HEAD(&mt->geom->lg_provider, mt->provider,
lg_provider);
@@ -162,6 +178,10 @@
mt = userData;
sbuf_finish(mt->sbuf[mt->level]);
p = strdup(sbuf_data(mt->sbuf[mt->level]));
+ if (p == NULL) {
+ warn("Problem during processing of '%s' element", name);
+ return;
+ }
sbuf_delete(mt->sbuf[mt->level]);
mt->sbuf[mt->level] = NULL;
mt->level--;
@@ -212,8 +232,16 @@
}
if (mt->config != NULL) {
- gc = calloc(sizeof *gc, 1);
+ gc = calloc(1, sizeof *gc);
+ if (gc == NULL) {
+ warn("Problem during processing of '%s' element", name);
+ return;
+ }
gc->lg_name = strdup(name);
+ if (gc->lg_name == NULL) {
+ warn("Problem during processing of '%s' element", name);
+ return;
+ }
gc->lg_val = p;
LIST_INSERT_HEAD(mt->config, gc, lg_config);
return;
--------------070408060806070200010105--
More information about the freebsd-bugs
mailing list