On 08/09/18 11:40, Theo Buehler wrote:
On Sat, Sep 08, 2018 at 11:07:30AM +0200, Anton Lindqvist wrote:
On Fri, Sep 07, 2018 at 09:22:33PM +0200, Theo Buehler wrote:
On Fri, Sep 07, 2018 at 09:15:30PM +0200, Clemens Goessnitzer wrote:
This patch adds 2 missing NULL pointer checks to rebound.c after malloc().
The same function also contains an unchecked calloc.
Since it's a daemon I guess it makes sense to continue execution instead
of dying. However, we should make sure to not leak memory along the
error paths. Also, log something when preloading the cache fails.
Regress passed; ok?
You may want to consider returning -1 instead of 1 on failure for
consistency with the rest of the daemon. Apart from that this looks
good.
ok
Or maybe something with goto error?
Clemens
Index: rebound.c
===================================================================
RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
retrieving revision 1.98
diff -u -p -r1.98 rebound.c
--- rebound.c 1 May 2018 15:14:43 -0000 1.98
+++ rebound.c 8 Sep 2018 09:45:23 -0000
@@ -630,12 +630,12 @@ encodename(const char *name, unsigned ch
return totlen;
}
-static void
+static int
preloadcache(const char *name, uint16_t type, void *rdata, uint16_t rdatalen)
{
- struct dnspacket *req, *resp;
+ struct dnspacket *req = NULL, *resp = NULL;
size_t reqlen, resplen;
- struct dnscache *ent;
+ struct dnscache *ent = NULL;
unsigned char *p;
uint16_t len, class;
uint32_t ttl;
@@ -643,6 +643,8 @@ preloadcache(const char *name, uint16_t
/* header + len + name + type + class */
reqlen = sizeof(*req) + 1 + strlen(name) + 2 + 2;
req = malloc(reqlen);
+ if (req == NULL)
+ goto error;
req->id = 0;
req->flags = htons(0x100);
@@ -661,6 +663,9 @@ preloadcache(const char *name, uint16_t
/* req + name (compressed) + type + class + ttl + len + data */
resplen = reqlen + 2 + 2 + 2 + 4 + 2 + rdatalen;
resp = malloc(resplen);
+ if (resp == NULL)
+ goto error;
+
memcpy(resp, req, reqlen);
resp->flags = htons(0x100 | 0x8000); /* response */
resp->ancount = htons(1);
@@ -682,6 +687,9 @@ preloadcache(const char *name, uint16_t
memcpy(p, rdata, rdatalen);
ent = calloc(1, sizeof(*ent));
+ if (ent == NULL)
+ goto error;
+
ent->req = req;
ent->reqlen = reqlen;
ent->resp = resp;
@@ -690,6 +698,13 @@ preloadcache(const char *name, uint16_t
RB_INSERT(cachetree, &cachetree, ent);
+ return 0;
+
+error:
+ free(req);
+ free(resp);
+ free(ent);
+ return -1;
}
static void
@@ -699,7 +714,8 @@ preloadA(const char *name, const char *i
inet_aton(ip, &in);
- preloadcache(name, htons(1), &in, 4);
+ if (preloadcache(name, htons(1), &in, 4) == -1)
+ logerr("failed to add cahce entry for %s", name);
}
static void
@@ -715,7 +731,8 @@ preloadPTR(const char *ip, const char *n
encodename(name, namebuf);
- preloadcache(ipbuf, htons(12), namebuf, 1 + strlen(namebuf));
+ if (preloadcache(ipbuf, htons(12), namebuf, 1 + strlen(namebuf)) == -1)
+ logerr("failed to load cache entry for %s", ip);
}
static int
