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")
This commit is contained in:
Kristof Provost 2024-05-09 13:52:22 +02:00
parent ee72bc1d1f
commit f1612e7087
6 changed files with 33 additions and 14 deletions

View File

@ -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)

View File

@ -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,

View File

@ -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;

View File

@ -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);
}

View File

@ -75,6 +75,7 @@ struct pfr_buffer; /* forward definition */
struct pfctl {
int dev;
struct pfctl_handle *h;
int opts;
int optimize;
int loadopt;

View File

@ -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;