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?
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:05:28 -0000
@@ -630,7 +630,7 @@ 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;
@@ -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)
+ return 1;
req->id = 0;
req->flags = htons(0x100);
@@ -661,6 +663,10 @@ 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) {
+ free(req);
+ return 1;
+ }
memcpy(resp, req, reqlen);
resp->flags = htons(0x100 | 0x8000); /* response */
resp->ancount = htons(1);
@@ -682,6 +688,11 @@ preloadcache(const char *name, uint16_t
memcpy(p, rdata, rdatalen);
ent = calloc(1, sizeof(*ent));
+ if (ent == NULL) {
+ free(req);
+ free(resp);
+ return 1;
+ }
ent->req = req;
ent->reqlen = reqlen;
ent->resp = resp;
@@ -689,7 +700,7 @@ preloadcache(const char *name, uint16_t
ent->permanent = 1;
RB_INSERT(cachetree, &cachetree, ent);
-
+ return 0;
}
static void
@@ -699,7 +710,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))
+ logerr("failed to add cache entry for %s", name);
}
static void
@@ -715,7 +727,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)))
+ logerr("failed to add cache entry for %s", ip);
}
static int