From nobody Thu May 09 12:10:22 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VZrSH0DHXz5JWvf; Thu, 09 May 2024 12:10:23 +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 4VZrSG6bZ7z4yrw; Thu, 9 May 2024 12:10:22 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1715256622; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=nd6Dn3XHd3SURnTWsj4V3OvHhtgmir3cKq4tYCYEoFs=; b=wKRvvvmYqCLNhzi/vlULkbWfZVclJW62WY+EGsHAM8qxb+5w5et7BsIOwPEp1lK3/7qRtJ c91ocyEwG5+Q660OpUgDfK5a6dN2qDWfptpORykegmG9dDoJDu+z+IznRVZjSs9Nez/hbK wxOUp9JHteU5FO8Gp5Nk4yWB4Has8Y+0QyQcUx44sJkjiqgxIzInf0mOXeqEVT3nIpUyPU R8EVkeyy6I+79YVfms5/qC9yaKp/hvyRkD5M1r3XzIcsh2Gx+xqg1ulPWauinnfdo7qkSK bDCTY5gZTSyhV8ke97zzY++koRh1ALuRUHHq8mUaN+siIzHkACQYHctwojrptQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1715256622; a=rsa-sha256; cv=none; b=tyWmtc7Cv7tVIlxxNNCJbM/GznY1PGY/fLKyVDnnRVxqMXumc5Bqk+bXmelBApvaPb73a+ c5YJXC27nzV90BDn4YMCxEL8KXDR+8mrjbM42wzJqU1Sx8EaG1aPr5XAONR7L6Q7Xa+5AM oYvvN+y58AlG2L4R2JZDPkPSmHBSzg/NFr7nmTQSr3RVEJkPoutxSmjS+nK12Qv1hAcRoG i3MYWITsqFKQp2cYTHhbtG36o44rmdSFO0aFUoLteQMc4njFyodEEkOdwWw/93y+glKO/M 7IegBkWcXKttLKgzXPXCk6eukOZJ8ux4Bq56TYqberVekigFPNHhjh7TCAELww== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1715256622; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=nd6Dn3XHd3SURnTWsj4V3OvHhtgmir3cKq4tYCYEoFs=; b=PkGdItMvLgIXfaILt+L3PcV6hblfJbDdWEjv6cCtDfEIZnvkWqi2ArK6+n0Md7GfyIKIeY j9VUM8B+xsbWqKrh5VylMkqKGVAmQCIxZVjWEk3v98wVI1Cx5i2f+UUU9znJE9Cp6EXKxa 7aMeziEEFG8ua+0b4BzKsq7C7wCsrRspG9TgVzwZ3RDaaRwZtW8yQ9GY6PM6t5BbyO5sSP Xv82wAmQTsI2nO2hEainx9NJxfgbpyxe6a8PadelfygZMoidTL4blcHsM3PJFZ1Jt5Qj2B R4ggW1Y0EPc1up4I885G19IZkrWlDwuT0JPk4y0ZeecKVV2SrpYqfx2RgwvdbQ== 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 4VZrSG6Bs9z17Xs; Thu, 9 May 2024 12:10:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 449CAM0T023570; Thu, 9 May 2024 12:10:22 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 449CAMk0023567; Thu, 9 May 2024 12:10:22 GMT (envelope-from git) Date: Thu, 9 May 2024 12:10:22 GMT Message-Id: <202405091210.449CAMk0023567@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: f1612e7087d7 - main - libpfctl: fix file descriptor leak List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org 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: f1612e7087d7c3df766ff0bf58c48d02fb0e2f6d Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=f1612e7087d7c3df766ff0bf58c48d02fb0e2f6d commit f1612e7087d7c3df766ff0bf58c48d02fb0e2f6d Author: Kristof Provost AuthorDate: 2024-05-09 11:52:22 +0000 Commit: Kristof Provost CommitDate: 2024-05-09 12:07:07 +0000 libpfctl: fix file descriptor leak pfctl_get_rules_info() opened a netlink socket, but failed to close it again. Fix this by factoring out the netlink-based function into a _h variant that takes struct pfctl_handle, and implement pfctl_get_rules_info() based on that, remembering to close the fd. While here migrate all in-tree consumers to the _h variant. MFC after: 3 days Sponsored by: Rubicon Communications, LLC ("Netgate") --- lib/libpfctl/libpfctl.c | 30 ++++++++++++++++++++++-------- lib/libpfctl/libpfctl.h | 3 +++ sbin/pfctl/pfctl.c | 9 +++++---- sbin/pfctl/pfctl_optimize.c | 2 +- sbin/pfctl/pfctl_parser.h | 1 + usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c | 2 +- 6 files changed, 33 insertions(+), 14 deletions(-) diff --git a/lib/libpfctl/libpfctl.c b/lib/libpfctl/libpfctl.c index 6da3b6969107..479b96123012 100644 --- a/lib/libpfctl/libpfctl.c +++ b/lib/libpfctl/libpfctl.c @@ -1336,22 +1336,20 @@ static struct snl_field_parser fp_getrules[] = { SNL_DECLARE_PARSER(getrules_parser, struct genlmsghdr, fp_getrules, ap_getrules); int -pfctl_get_rules_info(int dev __unused, struct pfctl_rules_info *rules, uint32_t ruleset, +pfctl_get_rules_info_h(struct pfctl_handle *h, struct pfctl_rules_info *rules, uint32_t ruleset, const char *path) { - struct snl_state ss = {}; struct snl_errmsg_data e = {}; struct nlmsghdr *hdr; struct snl_writer nw; uint32_t seq_id; int family_id; - snl_init(&ss, NETLINK_GENERIC); - family_id = snl_get_genl_family(&ss, PFNL_FAMILY_NAME); + family_id = snl_get_genl_family(&h->ss, PFNL_FAMILY_NAME); if (family_id == 0) return (ENOTSUP); - snl_init_writer(&ss, &nw); + snl_init_writer(&h->ss, &nw); hdr = snl_create_genl_msg_request(&nw, family_id, PFNL_CMD_GETRULES); hdr->nlmsg_flags |= NLM_F_DUMP; @@ -1363,17 +1361,33 @@ pfctl_get_rules_info(int dev __unused, struct pfctl_rules_info *rules, uint32_t return (ENOMEM); seq_id = hdr->nlmsg_seq; - if (! snl_send_message(&ss, hdr)) + if (! snl_send_message(&h->ss, hdr)) return (ENXIO); - while ((hdr = snl_read_reply_multi(&ss, seq_id, &e)) != NULL) { - if (! snl_parse_nlmsg(&ss, hdr, &getrules_parser, rules)) + while ((hdr = snl_read_reply_multi(&h->ss, seq_id, &e)) != NULL) { + if (! snl_parse_nlmsg(&h->ss, hdr, &getrules_parser, rules)) continue; } return (e.error); } +int +pfctl_get_rules_info(int dev __unused, struct pfctl_rules_info *rules, uint32_t ruleset, + const char *path) +{ + struct pfctl_handle *h; + int error; + + h = pfctl_open(PF_DEVICE); + if (h == NULL) + return (ENOTSUP); + error = pfctl_get_rules_info_h(h, rules, ruleset, path); + pfctl_close(h); + + return (error); +} + int pfctl_get_rule(int dev, uint32_t nr, uint32_t ticket, const char *anchor, uint32_t ruleset, struct pfctl_rule *rule, char *anchor_call) diff --git a/lib/libpfctl/libpfctl.h b/lib/libpfctl/libpfctl.h index 2937a36a8a47..73282eb3cc3d 100644 --- a/lib/libpfctl/libpfctl.h +++ b/lib/libpfctl/libpfctl.h @@ -412,6 +412,9 @@ int pfctl_get_eth_rule(int dev, uint32_t nr, uint32_t ticket, char *anchor_call); int pfctl_add_eth_rule(int dev, const struct pfctl_eth_rule *r, const char *anchor, const char *anchor_call, uint32_t ticket); +int pfctl_get_rules_info_h(struct pfctl_handle *h, + struct pfctl_rules_info *rules, uint32_t ruleset, + const char *path); int pfctl_get_rules_info(int dev, struct pfctl_rules_info *rules, uint32_t ruleset, const char *path); int pfctl_get_rule(int dev, uint32_t nr, uint32_t ticket, diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index 17901b04a130..19d05c415f02 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -1283,14 +1283,14 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, } if (opts & PF_OPT_SHOWALL) { - ret = pfctl_get_rules_info(dev, &ri, PF_PASS, path); + ret = pfctl_get_rules_info_h(pfh, &ri, PF_PASS, path); if (ret != 0) { warn("DIOCGETRULES"); goto error; } header++; } - ret = pfctl_get_rules_info(dev, &ri, PF_SCRUB, path); + ret = pfctl_get_rules_info_h(pfh, &ri, PF_SCRUB, path); if (ret != 0) { warn("DIOCGETRULES"); goto error; @@ -1328,7 +1328,7 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, } pfctl_clear_pool(&rule.rpool); } - ret = pfctl_get_rules_info(dev, &ri, PF_PASS, path); + ret = pfctl_get_rules_info_h(pfh, &ri, PF_PASS, path); if (ret != 0) { warn("DIOCGETRULES"); goto error; @@ -1435,7 +1435,7 @@ pfctl_show_nat(int dev, char *path, int opts, char *anchorname, int depth) snprintf(&path[len], MAXPATHLEN - len, "%s", anchorname); for (i = 0; i < 3; i++) { - ret = pfctl_get_rules_info(dev, &ri, nattype[i], path); + ret = pfctl_get_rules_info_h(pfh, &ri, nattype[i], path); if (ret != 0) { warn("DIOCGETRULES"); return (-1); @@ -2130,6 +2130,7 @@ pfctl_rules(int dev, char *filename, int opts, int optimize, sizeof(trs.pfrt_anchor)) >= sizeof(trs.pfrt_anchor)) ERRX("pfctl_rules: strlcpy"); pf.dev = dev; + pf.h = pfh; pf.opts = opts; pf.optimize = optimize; pf.loadopt = loadopt; diff --git a/sbin/pfctl/pfctl_optimize.c b/sbin/pfctl/pfctl_optimize.c index 95292999c50a..9b43a840c06f 100644 --- a/sbin/pfctl/pfctl_optimize.c +++ b/sbin/pfctl/pfctl_optimize.c @@ -889,7 +889,7 @@ load_feedback_profile(struct pfctl *pf, struct superblocks *superblocks) TAILQ_INIT(&queue); TAILQ_INIT(&prof_superblocks); - if (pfctl_get_rules_info(pf->dev, &rules, PF_PASS, "")) { + if (pfctl_get_rules_info_h(pf->h, &rules, PF_PASS, "")) { warn("DIOCGETRULES"); return (1); } diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h index d0f3bc3c303c..6534baa9a7dd 100644 --- a/sbin/pfctl/pfctl_parser.h +++ b/sbin/pfctl/pfctl_parser.h @@ -75,6 +75,7 @@ struct pfr_buffer; /* forward definition */ struct pfctl { int dev; + struct pfctl_handle *h; int opts; int optimize; int loadopt; diff --git a/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c b/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c index e618683845be..1086aa7dcf82 100644 --- a/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c +++ b/usr.sbin/bsnmpd/modules/snmp_pf/pf_snmp.c @@ -1519,7 +1519,7 @@ pfl_scan_ruleset(const char *path) struct pfl_entry *e; u_int32_t nr, i; - if (pfctl_get_rules_info(pfctl_fd(pfh), &rules, PF_PASS, path)) { + if (pfctl_get_rules_info_h(pfh, &rules, PF_PASS, path)) { syslog(LOG_ERR, "pfl_scan_ruleset: ioctl(DIOCGETRULES): %s", strerror(errno)); goto err;