git: f1612e7087d7 - main - libpfctl: fix file descriptor leak
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 09 May 2024 12:10:22 UTC
The branch main has been updated by kp:
URL: https://cgit.FreeBSD.org/src/commit/?id=f1612e7087d7c3df766ff0bf58c48d02fb0e2f6d
commit f1612e7087d7c3df766ff0bf58c48d02fb0e2f6d
Author: Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-05-09 11:52:22 +0000
Commit: Kristof Provost <kp@FreeBSD.org>
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;