From owner-dev-commits-src-all@freebsd.org Thu May 27 10:20:39 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 7D2016386AC; Thu, 27 May 2021 10:20:39 +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 4FrP3B0xXnz4sCK; Thu, 27 May 2021 10:20:38 +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 66213224A4; Thu, 27 May 2021 10:20:35 +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 14RAKZXX055662; Thu, 27 May 2021 10:20:35 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 14RAKZcN055661; Thu, 27 May 2021 10:20:35 GMT (envelope-from git) Date: Thu, 27 May 2021 10:20:35 GMT Message-Id: <202105271020.14RAKZcN055661@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kristof Provost Subject: git: ff4447ac31ca - stable/13 - 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/stable/13 X-Git-Reftype: branch X-Git-Commit: ff4447ac31ca1ee54ac7e2a01ba11c3bc8cafdca 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: Thu, 27 May 2021 10:20:39 -0000 The branch stable/13 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=ff4447ac31ca1ee54ac7e2a01ba11c3bc8cafdca commit ff4447ac31ca1ee54ac7e2a01ba11c3bc8cafdca Author: Kristof Provost AuthorDate: 2021-05-24 06:32:16 +0000 Commit: Kristof Provost CommitDate: 2021-05-27 07:12:03 +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") (cherry picked from commit 4483fb47735c29408c72045469c9c4b3e549668b) --- 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 849662a23aa5..ea4924e2dd41 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); @@ -2670,6 +2682,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); @@ -2706,26 +2719,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)