From 0cc1b06d98676ab66200d388c48c3cd615aa0109 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 9 Dec 2022 12:41:38 +0000 Subject: [PATCH 1/2] Fix dns_fwdtable_addfwd() error path cleanup bug Free 'sizeof(dns_forwarder_t)' bytes of memory instead of 'sizeof(dns_sockaddr_t)' bytes, because `fwd` is a pointer to a 'dns_forwarder_t' type structure. --- lib/dns/forward.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns/forward.c b/lib/dns/forward.c index ea91fb25a9..823331dc46 100644 --- a/lib/dns/forward.c +++ b/lib/dns/forward.c @@ -103,7 +103,7 @@ cleanup: while (!ISC_LIST_EMPTY(forwarders->fwdrs)) { fwd = ISC_LIST_HEAD(forwarders->fwdrs); ISC_LIST_UNLINK(forwarders->fwdrs, fwd, link); - isc_mem_put(fwdtable->mctx, fwd, sizeof(isc_sockaddr_t)); + isc_mem_put(fwdtable->mctx, fwd, sizeof(dns_forwarder_t)); } isc_mem_put(fwdtable->mctx, forwarders, sizeof(dns_forwarders_t)); return (result); From cf4003fa5801230a7e844e2abe5878520a58a748 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Mon, 12 Dec 2022 09:20:48 +0000 Subject: [PATCH 2/2] Use sizeof(*ptr) for allocating/freeing memory in forward.c As shown in the previous commit, using sizeof(type_t) is a little bit more error-prone when copy-pasting code, so extracting the size information from the pointer which is being dealt with seems like a better alternative. --- lib/dns/forward.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/dns/forward.c b/lib/dns/forward.c index 823331dc46..8750569c5d 100644 --- a/lib/dns/forward.c +++ b/lib/dns/forward.c @@ -45,7 +45,7 @@ dns_fwdtable_create(isc_mem_t *mctx, dns_fwdtable_t **fwdtablep) { REQUIRE(fwdtablep != NULL && *fwdtablep == NULL); - fwdtable = isc_mem_get(mctx, sizeof(dns_fwdtable_t)); + fwdtable = isc_mem_get(mctx, sizeof(*fwdtable)); fwdtable->table = NULL; result = dns_rbt_create(mctx, auto_detach, fwdtable, &fwdtable->table); @@ -62,7 +62,7 @@ dns_fwdtable_create(isc_mem_t *mctx, dns_fwdtable_t **fwdtablep) { return (ISC_R_SUCCESS); cleanup_fwdtable: - isc_mem_put(mctx, fwdtable, sizeof(dns_fwdtable_t)); + isc_mem_put(mctx, fwdtable, sizeof(*fwdtable)); return (result); } @@ -76,13 +76,13 @@ dns_fwdtable_addfwd(dns_fwdtable_t *fwdtable, const dns_name_t *name, REQUIRE(VALID_FWDTABLE(fwdtable)); - forwarders = isc_mem_get(fwdtable->mctx, sizeof(dns_forwarders_t)); + forwarders = isc_mem_get(fwdtable->mctx, sizeof(*forwarders)); ISC_LIST_INIT(forwarders->fwdrs); for (fwd = ISC_LIST_HEAD(*fwdrs); fwd != NULL; fwd = ISC_LIST_NEXT(fwd, link)) { - nfwd = isc_mem_get(fwdtable->mctx, sizeof(dns_forwarder_t)); + nfwd = isc_mem_get(fwdtable->mctx, sizeof(*nfwd)); *nfwd = *fwd; ISC_LINK_INIT(nfwd, link); ISC_LIST_APPEND(forwarders->fwdrs, nfwd, link); @@ -103,9 +103,9 @@ cleanup: while (!ISC_LIST_EMPTY(forwarders->fwdrs)) { fwd = ISC_LIST_HEAD(forwarders->fwdrs); ISC_LIST_UNLINK(forwarders->fwdrs, fwd, link); - isc_mem_put(fwdtable->mctx, fwd, sizeof(dns_forwarder_t)); + isc_mem_put(fwdtable->mctx, fwd, sizeof(*fwd)); } - isc_mem_put(fwdtable->mctx, forwarders, sizeof(dns_forwarders_t)); + isc_mem_put(fwdtable->mctx, forwarders, sizeof(*forwarders)); return (result); } @@ -119,13 +119,13 @@ dns_fwdtable_add(dns_fwdtable_t *fwdtable, const dns_name_t *name, REQUIRE(VALID_FWDTABLE(fwdtable)); - forwarders = isc_mem_get(fwdtable->mctx, sizeof(dns_forwarders_t)); + forwarders = isc_mem_get(fwdtable->mctx, sizeof(*forwarders)); ISC_LIST_INIT(forwarders->fwdrs); for (sa = ISC_LIST_HEAD(*addrs); sa != NULL; sa = ISC_LIST_NEXT(sa, link)) { - fwd = isc_mem_get(fwdtable->mctx, sizeof(dns_forwarder_t)); + fwd = isc_mem_get(fwdtable->mctx, sizeof(*fwd)); fwd->addr = *sa; ISC_LINK_INIT(fwd, link); ISC_LIST_APPEND(forwarders->fwdrs, fwd, link); @@ -146,9 +146,9 @@ cleanup: while (!ISC_LIST_EMPTY(forwarders->fwdrs)) { fwd = ISC_LIST_HEAD(forwarders->fwdrs); ISC_LIST_UNLINK(forwarders->fwdrs, fwd, link); - isc_mem_put(fwdtable->mctx, fwd, sizeof(dns_forwarder_t)); + isc_mem_put(fwdtable->mctx, fwd, sizeof(*fwd)); } - isc_mem_put(fwdtable->mctx, forwarders, sizeof(dns_forwarders_t)); + isc_mem_put(fwdtable->mctx, forwarders, sizeof(*forwarders)); return (result); } @@ -193,7 +193,7 @@ dns_fwdtable_destroy(dns_fwdtable_t **fwdtablep) { isc_rwlock_destroy(&fwdtable->rwlock); fwdtable->magic = 0; - isc_mem_putanddetach(&fwdtable->mctx, fwdtable, sizeof(dns_fwdtable_t)); + isc_mem_putanddetach(&fwdtable->mctx, fwdtable, sizeof(*fwdtable)); } /*** @@ -211,7 +211,7 @@ auto_detach(void *data, void *arg) { while (!ISC_LIST_EMPTY(forwarders->fwdrs)) { fwd = ISC_LIST_HEAD(forwarders->fwdrs); ISC_LIST_UNLINK(forwarders->fwdrs, fwd, link); - isc_mem_put(fwdtable->mctx, fwd, sizeof(dns_forwarder_t)); + isc_mem_put(fwdtable->mctx, fwd, sizeof(*fwd)); } - isc_mem_put(fwdtable->mctx, forwarders, sizeof(dns_forwarders_t)); + isc_mem_put(fwdtable->mctx, forwarders, sizeof(*forwarders)); }