From owner-dev-commits-src-all@freebsd.org Mon May 24 16:45:36 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id F031B6386D2; Mon, 24 May 2021 16:45:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Fpjkm3zMgz4WK6; Mon, 24 May 2021 16:45:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 57AEA5559; Mon, 24 May 2021 16:45:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 14OGjaBH005674; Mon, 24 May 2021 16:45:36 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 14OGjasN005673; Mon, 24 May 2021 16:45:36 GMT (envelope-from git) Date: Mon, 24 May 2021 16:45:36 GMT Message-Id: <202105241645.14OGjasN005673@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 4483fb47735c - main - pf: fix ioctl() memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 4483fb47735c29408c72045469c9c4b3e549668b Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 May 2021 16:45:37 -0000 The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=4483fb47735c29408c72045469c9c4b3e549668b commit 4483fb47735c29408c72045469c9c4b3e549668b Author: Kristof Provost AuthorDate: 2021-05-24 06:32:16 +0000 Commit: Kristof Provost CommitDate: 2021-05-24 13:56:24 +0000 pf: fix ioctl() memory leak When we create an nvlist and insert it into another nvlist we must remember to destroy it. The nvlist_add_nvlist() function makes a copy, just like nvlist_add_string() makes a copy of the string. If we don't we're leaking memory on every (nvlist-based) ioctl() call. While here remove two redundant 'break' statements. PR: 255971 MFC after: 3 days Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/netpfil/pf/pf_ioctl.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index 91a175caa74f..5686a93f6e52 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -1782,6 +1782,7 @@ pf_pool_to_nvpool(const struct pf_kpool *pool) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "counter", tmp); + nvlist_destroy(tmp); nvlist_add_number(nvl, "tblidx", pool->tblidx); pf_uint16_array_nv(nvl, "proxy_port", pool->proxy_port, 2); @@ -1791,6 +1792,7 @@ pf_pool_to_nvpool(const struct pf_kpool *pool) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "mape", tmp); + nvlist_destroy(tmp); return (nvl); @@ -1862,10 +1864,12 @@ pf_addr_wrap_to_nvaddr_wrap(const struct pf_addr_wrap *addr) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "addr", tmp); + nvlist_destroy(tmp); tmp = pf_addr_to_nvaddr(&addr->v.a.mask); if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "mask", tmp); + nvlist_destroy(tmp); return (nvl); @@ -1930,6 +1934,7 @@ pf_rule_addr_to_nvrule_addr(const struct pf_rule_addr *addr) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "addr", tmp); + nvlist_destroy(tmp); pf_uint16_array_nv(nvl, "port", addr->port, 2); nvlist_add_number(nvl, "neg", addr->neg); nvlist_add_number(nvl, "port_op", addr->port_op); @@ -2155,6 +2160,7 @@ pf_divert_to_nvdivert(const struct pf_krule *rule) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "addr", tmp); + nvlist_destroy(tmp); nvlist_add_number(nvl, "port", rule->divert.port); return (nvl); @@ -2178,10 +2184,12 @@ pf_krule_to_nvrule(const struct pf_krule *rule) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "src", tmp); + nvlist_destroy(tmp); tmp = pf_rule_addr_to_nvrule_addr(&rule->dst); if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "dst", tmp); + nvlist_destroy(tmp); for (int i = 0; i < PF_SKIP_COUNT; i++) { nvlist_append_number_array(nvl, "skip", @@ -2203,6 +2211,7 @@ pf_krule_to_nvrule(const struct pf_krule *rule) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "rpool", tmp); + nvlist_destroy(tmp); nvlist_add_number(nvl, "evaluations", counter_u64_fetch(rule->evaluations)); @@ -2248,10 +2257,12 @@ pf_krule_to_nvrule(const struct pf_krule *rule) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "uid", tmp); + nvlist_destroy(tmp); tmp = pf_rule_uid_to_nvrule_uid((const struct pf_rule_uid *)&rule->gid); if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "gid", tmp); + nvlist_destroy(tmp); nvlist_add_number(nvl, "rule_flag", rule->rule_flag); nvlist_add_number(nvl, "action", rule->action); @@ -2288,6 +2299,7 @@ pf_krule_to_nvrule(const struct pf_krule *rule) if (tmp == NULL) goto error; nvlist_add_nvlist(nvl, "divert", tmp); + nvlist_destroy(tmp); return (nvl); @@ -2532,6 +2544,7 @@ pf_state_peer_to_nvstate_peer(const struct pf_state_peer *peer) if (tmp == NULL) goto errout; nvlist_add_nvlist(nvl, "scrub", tmp); + nvlist_destroy(tmp); } nvlist_add_number(nvl, "seqlo", peer->seqlo); @@ -2568,26 +2581,31 @@ pf_state_to_nvstate(const struct pf_state *s) if (tmp == NULL) goto errout; nvlist_add_nvlist(nvl, "stack_key", tmp); + nvlist_destroy(tmp); tmp = pf_state_key_to_nvstate_key(s->key[PF_SK_WIRE]); if (tmp == NULL) goto errout; nvlist_add_nvlist(nvl, "wire_key", tmp); + nvlist_destroy(tmp); tmp = pf_state_peer_to_nvstate_peer(&s->src); if (tmp == NULL) goto errout; nvlist_add_nvlist(nvl, "src", tmp); + nvlist_destroy(tmp); tmp = pf_state_peer_to_nvstate_peer(&s->dst); if (tmp == NULL) goto errout; nvlist_add_nvlist(nvl, "dst", tmp); + nvlist_destroy(tmp); tmp = pf_addr_to_nvaddr(&s->rt_addr); if (tmp == NULL) goto errout; nvlist_add_nvlist(nvl, "rt_addr", tmp); + nvlist_destroy(tmp); nvlist_add_number(nvl, "rule", s->rule.ptr ? s->rule.ptr->nr : -1); nvlist_add_number(nvl, "anchor", @@ -3317,7 +3335,6 @@ DIOCADDRULENV_error: ruleset->rules[rs_num].active.ticket) { PF_RULES_WUNLOCK(); ERROUT(EBUSY); - break; } if ((error = nvlist_error(nvl))) { @@ -3331,7 +3348,6 @@ DIOCADDRULENV_error: if (rule == NULL) { PF_RULES_WUNLOCK(); ERROUT(EBUSY); - break; } nvrule = pf_krule_to_nvrule(rule); @@ -3344,6 +3360,7 @@ DIOCADDRULENV_error: } nvlist_add_number(nvl, "nr", nr); nvlist_add_nvlist(nvl, "rule", nvrule); + nvlist_destroy(nvrule); nvrule = NULL; if (pf_kanchor_nvcopyout(ruleset, rule, nvl)) { PF_RULES_WUNLOCK(); @@ -5912,6 +5929,7 @@ pf_getstate(struct pfioc_nv *nv) ERROUT(ENOMEM); nvlist_add_nvlist(nvl, "state", nvls); + nvlist_destroy(nvls); nvlpacked = nvlist_pack(nvl, &nv->len); if (nvlpacked == NULL)