The branch main has been updated by kp:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=375aaa299f85a66cee808490c31809db9e890a68

commit 375aaa299f85a66cee808490c31809db9e890a68
Author:     Kristof Provost <[email protected]>
AuthorDate: 2024-07-26 19:03:54 +0000
Commit:     Kristof Provost <[email protected]>
CommitDate: 2024-07-29 17:42:25 +0000

    pfctl: improve error reporting
    
    libpfctl doesn't set errno, instead it returns error codes. Take that into
    account when handling errors so that we report the actual error.
    
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
---
 sbin/pfctl/pfctl.c | 140 +++++++++++++++++++++++++++++------------------------
 1 file changed, 77 insertions(+), 63 deletions(-)

diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 39c6d684a317..b60e64fba338 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -319,7 +319,7 @@ pfctl_enable(int dev, int opts)
                else if (ret == ESRCH)
                        errx(1, "pfil registeration failed");
                else
-                       err(1, "DIOCSTART");
+                       errc(1, ret, "DIOCSTART");
        }
        if ((opts & PF_OPT_QUIET) == 0)
                fprintf(stderr, "pf enabled\n");
@@ -340,7 +340,7 @@ pfctl_disable(int dev, int opts)
                if (ret == ENOENT)
                        errx(1, "pf not enabled");
                else
-                       err(1, "DIOCSTOP");
+                       errc(1, ret, "DIOCSTOP");
        }
        if ((opts & PF_OPT_QUIET) == 0)
                fprintf(stderr, "pf disabled\n");
@@ -355,8 +355,9 @@ pfctl_disable(int dev, int opts)
 int
 pfctl_clear_stats(struct pfctl_handle *h, int opts)
 {
-       if (pfctl_clear_status(h))
-               err(1, "DIOCCLRSTATUS");
+       int ret;
+       if ((ret = pfctl_clear_status(h)) != 0)
+               errc(1, ret, "DIOCCLRSTATUS");
        if ((opts & PF_OPT_QUIET) == 0)
                fprintf(stderr, "pf: statistics cleared\n");
        return (0);
@@ -536,6 +537,7 @@ pfctl_clear_iface_states(int dev, const char *iface, int 
opts)
 {
        struct pfctl_kill kill;
        unsigned int killed;
+       int ret;
 
        memset(&kill, 0, sizeof(kill));
        if (iface != NULL && strlcpy(kill.ifname, iface,
@@ -545,8 +547,8 @@ pfctl_clear_iface_states(int dev, const char *iface, int 
opts)
        if (opts & PF_OPT_KILLMATCH)
                kill.kill_match = true;
 
-       if (pfctl_clear_states_h(pfh, &kill, &killed))
-               err(1, "DIOCCLRSTATES");
+       if ((ret = pfctl_clear_states_h(pfh, &kill, &killed)) != 0)
+               errc(1, ret, "DIOCCLRSTATES");
        if ((opts & PF_OPT_QUIET) == 0)
                fprintf(stderr, "%d states cleared\n", killed);
        return (0);
@@ -713,7 +715,7 @@ pfctl_net_kill_states(int dev, const char *iface, int opts)
        struct sockaddr last_src, last_dst;
        unsigned int newkilled;
        int killed, sources, dests;
-       int ret_ga;
+       int ret_ga, ret;
 
        killed = sources = dests = 0;
 
@@ -801,14 +803,14 @@ pfctl_net_kill_states(int dev, const char *iface, int 
opts)
                                        errx(1, "Unknown address family %d",
                                            kill.af);
 
-                               if (pfctl_kill_states_h(pfh, &kill, &newkilled))
-                                       err(1, "DIOCKILLSTATES");
+                               if ((ret = pfctl_kill_states_h(pfh, &kill, 
&newkilled)) != 0)
+                                       errc(1, ret, "DIOCKILLSTATES");
                                killed += newkilled;
                        }
                        freeaddrinfo(res[1]);
                } else {
-                       if (pfctl_kill_states_h(pfh, &kill, &newkilled))
-                               err(1, "DIOCKILLSTATES");
+                       if ((ret = pfctl_kill_states_h(pfh, &kill, &newkilled)) 
!= 0)
+                               errc(1, ret, "DIOCKILLSTATES");
                        killed += newkilled;
                }
        }
@@ -890,6 +892,7 @@ pfctl_label_kill_states(int dev, const char *iface, int 
opts)
 {
        struct pfctl_kill kill;
        unsigned int killed;
+       int ret;
 
        if (state_killers != 2 || (strlen(state_kill[1]) == 0)) {
                warnx("no label specified");
@@ -907,8 +910,8 @@ pfctl_label_kill_states(int dev, const char *iface, int 
opts)
            sizeof(kill.label))
                errx(1, "label too long: %s", state_kill[1]);
 
-       if (pfctl_kill_states_h(pfh, &kill, &killed))
-               err(1, "DIOCKILLSTATES");
+       if ((ret = pfctl_kill_states_h(pfh, &kill, &killed)) != 0)
+               errc(1, ret, "DIOCKILLSTATES");
 
        if ((opts & PF_OPT_QUIET) == 0)
                fprintf(stderr, "killed %d states\n", killed);
@@ -921,6 +924,7 @@ pfctl_id_kill_states(int dev, const char *iface, int opts)
 {
        struct pfctl_kill kill;
        unsigned int killed;
+       int ret;
        
        if (state_killers != 2 || (strlen(state_kill[1]) == 0)) {
                warnx("no id specified");
@@ -946,8 +950,8 @@ pfctl_id_kill_states(int dev, const char *iface, int opts)
                usage();
        }
 
-       if (pfctl_kill_states_h(pfh, &kill, &killed))
-               err(1, "DIOCKILLSTATES");
+       if ((ret = pfctl_kill_states_h(pfh, &kill, &killed)) != 0)
+               errc(1, ret, "DIOCKILLSTATES");
 
        if ((opts & PF_OPT_QUIET) == 0)
                fprintf(stderr, "killed %d states\n", killed);
@@ -962,17 +966,18 @@ pfctl_get_pool(int dev, struct pfctl_pool *pool, 
u_int32_t nr,
        struct pfioc_pooladdr pp;
        struct pf_pooladdr *pa;
        u_int32_t pnr, mpnr;
+       int ret;
 
        memset(&pp, 0, sizeof(pp));
-       if (pfctl_get_addrs(pfh, ticket, nr, r_action, anchorname, &mpnr) != 0) 
{
-               warn("DIOCGETADDRS");
+       if ((ret = pfctl_get_addrs(pfh, ticket, nr, r_action, anchorname, 
&mpnr)) != 0) {
+               warnc(ret, "DIOCGETADDRS");
                return (-1);
        }
 
        TAILQ_INIT(&pool->list);
        for (pnr = 0; pnr < mpnr; ++pnr) {
-               if (pfctl_get_addr(pfh, ticket, nr, r_action, anchorname, pnr, 
&pp) != 0) {
-                       warn("DIOCGETADDR");
+               if ((ret = pfctl_get_addr(pfh, ticket, nr, r_action, 
anchorname, pnr, &pp)) != 0) {
+                       warnc(ret, "DIOCGETADDR");
                        return (-1);
                }
                pa = calloc(1, sizeof(struct pf_pooladdr));
@@ -1102,6 +1107,7 @@ pfctl_show_eth_rules(int dev, char *path, int opts, enum 
pfctl_show format,
        int brace;
        int dotitle = opts & PF_OPT_SHOWALL;
        int len = strlen(path);
+       int ret;
        char *npath, *p;
 
        /*
@@ -1134,12 +1140,12 @@ pfctl_show_eth_rules(int dev, char *path, int opts, 
enum pfctl_show format,
                struct pfctl_eth_rulesets_info  ri;
                u_int32_t                mnr, nr;
 
-               if (pfctl_get_eth_rulesets_info(dev, &ri, npath)) {
-                       if (errno == EINVAL) {
+               if ((ret = pfctl_get_eth_rulesets_info(dev, &ri, npath)) != 0) {
+                       if (ret == EINVAL) {
                                fprintf(stderr, "Anchor '%s' "
                                                "not found.\n", anchorname);
                        } else {
-                               warn("DIOCGETETHRULESETS");
+                               warnc(ret, "DIOCGETETHRULESETS");
                                return (-1);
                        }
                }
@@ -1149,8 +1155,8 @@ pfctl_show_eth_rules(int dev, char *path, int opts, enum 
pfctl_show format,
                for (nr = 0; nr < mnr; ++nr) {
                        struct pfctl_eth_ruleset_info   rs;
 
-                       if (pfctl_get_eth_ruleset(dev, npath, nr, &rs))
-                               err(1, "DIOCGETETHRULESET");
+                       if ((ret = pfctl_get_eth_ruleset(dev, npath, nr, &rs)) 
!= 0)
+                               errc(1, ret, "DIOCGETETHRULESET");
                        INDENT(depth, !(opts & PF_OPT_VERBOSE));
                        printf("anchor \"%s\" all {\n", rs.name);
                        pfctl_show_eth_rules(dev, npath, opts,
@@ -1162,16 +1168,16 @@ pfctl_show_eth_rules(int dev, char *path, int opts, 
enum pfctl_show format,
                return (0);
        }
 
-       if (pfctl_get_eth_rules_info(dev, &info, path)) {
-               warn("DIOCGETETHRULES");
+       if ((ret = pfctl_get_eth_rules_info(dev, &info, path)) != 0) {
+               warnc(ret, "DIOCGETETHRULES");
                return (-1);
        }
        for (int nr = 0; nr < info.nr; nr++) {
                brace = 0;
                INDENT(depth, !(opts & PF_OPT_VERBOSE));
-               if (pfctl_get_eth_rule(dev, nr, info.ticket, path, &rule,
-                   opts & PF_OPT_CLRRULECTRS, anchor_call) != 0) {
-                       warn("DIOCGETETHRULE");
+               if ((ret = pfctl_get_eth_rule(dev, nr, info.ticket, path, &rule,
+                   opts & PF_OPT_CLRRULECTRS, anchor_call)) != 0) {
+                       warnc(ret, "DIOCGETETHRULE");
                        return (-1);
                }
                if (anchor_call[0] &&
@@ -1280,14 +1286,14 @@ pfctl_show_rules(int dev, char *path, int opts, enum 
pfctl_show format,
        if (opts & PF_OPT_SHOWALL) {
                ret = pfctl_get_rules_info_h(pfh, &ri, PF_PASS, path);
                if (ret != 0) {
-                       warn("DIOCGETRULES");
+                       warnc(ret, "DIOCGETRULES");
                        goto error;
                }
                header++;
        }
        ret = pfctl_get_rules_info_h(pfh, &ri, PF_SCRUB, path);
        if (ret != 0) {
-               warn("DIOCGETRULES");
+               warnc(ret, "DIOCGETRULES");
                goto error;
        }
        if (opts & PF_OPT_SHOWALL) {
@@ -1298,9 +1304,9 @@ pfctl_show_rules(int dev, char *path, int opts, enum 
pfctl_show format,
        }
 
        for (nr = 0; nr < ri.nr; ++nr) {
-               if (pfctl_get_clear_rule_h(pfh, nr, ri.ticket, path, PF_SCRUB,
-                   &rule, anchor_call, opts & PF_OPT_CLRRULECTRS)) {
-                       warn("DIOCGETRULENV");
+               if ((ret = pfctl_get_clear_rule_h(pfh, nr, ri.ticket, path, 
PF_SCRUB,
+                   &rule, anchor_call, opts & PF_OPT_CLRRULECTRS)) != 0) {
+                       warnc(ret, "DIOCGETRULENV");
                        goto error;
                }
 
@@ -1325,13 +1331,13 @@ pfctl_show_rules(int dev, char *path, int opts, enum 
pfctl_show format,
        }
        ret = pfctl_get_rules_info_h(pfh, &ri, PF_PASS, path);
        if (ret != 0) {
-               warn("DIOCGETRULES");
+               warnc(ret, "DIOCGETRULES");
                goto error;
        }
        for (nr = 0; nr < ri.nr; ++nr) {
-               if (pfctl_get_clear_rule_h(pfh, nr, ri.ticket, path, PF_PASS,
-                   &rule, anchor_call, opts & PF_OPT_CLRRULECTRS)) {
-                       warn("DIOCGETRULE");
+               if ((ret = pfctl_get_clear_rule_h(pfh, nr, ri.ticket, path, 
PF_PASS,
+                   &rule, anchor_call, opts & PF_OPT_CLRRULECTRS)) != 0) {
+                       warnc(ret, "DIOCGETRULE");
                        goto error;
                }
 
@@ -1484,15 +1490,15 @@ pfctl_show_nat(int dev, char *path, int opts, char 
*anchorname, int depth,
        for (i = 0; i < 3; i++) {
                ret = pfctl_get_rules_info_h(pfh, &ri, nattype[i], path);
                if (ret != 0) {
-                       warn("DIOCGETRULES");
+                       warnc(ret, "DIOCGETRULES");
                        return (-1);
                }
                for (nr = 0; nr < ri.nr; ++nr) {
                        INDENT(depth, !(opts & PF_OPT_VERBOSE));
 
-                       if (pfctl_get_rule_h(pfh, nr, ri.ticket, path,
-                           nattype[i], &rule, anchor_call)) {
-                               warn("DIOCGETRULE");
+                       if ((ret = pfctl_get_rule_h(pfh, nr, ri.ticket, path,
+                           nattype[i], &rule, anchor_call)) != 0) {
+                               warnc(ret, "DIOCGETRULE");
                                return (-1);
                        }
                        if (pfctl_get_pool(dev, &rule.rpool, nr,
@@ -1613,14 +1619,15 @@ pfctl_show_status(int dev, int opts)
 {
        struct pfctl_status     *status;
        struct pfctl_syncookies cookies;
+       int ret;
 
        if ((status = pfctl_get_status_h(pfh)) == NULL) {
                warn("DIOCGETSTATUS");
                return (-1);
        }
-       if (pfctl_get_syncookies(dev, &cookies)) {
+       if ((ret = pfctl_get_syncookies(dev, &cookies)) != 0) {
                pfctl_free_status(status);
-               warn("DIOCGETSYNCOOKIES");
+               warnc(ret, "DIOCGETSYNCOOKIES");
                return (-1);
        }
        if (opts & PF_OPT_SHOWALL)
@@ -1653,12 +1660,13 @@ pfctl_show_timeouts(int dev, int opts)
 {
        uint32_t seconds;
        int i;
+       int ret;
 
        if (opts & PF_OPT_SHOWALL)
                pfctl_print_title("TIMEOUTS:");
        for (i = 0; pf_timeouts[i].name; i++) {
-               if (pfctl_get_timeout(pfh, pf_timeouts[i].timeout, &seconds))
-                       err(1, "DIOCGETTIMEOUT");
+               if ((ret = pfctl_get_timeout(pfh, pf_timeouts[i].timeout, 
&seconds)) != 0)
+                       errc(1, ret, "DIOCGETTIMEOUT");
                printf("%-20s %10d", pf_timeouts[i].name, seconds);
                if (pf_timeouts[i].timeout >= PFTM_ADAPTIVE_START &&
                    pf_timeouts[i].timeout <= PFTM_ADAPTIVE_END)
@@ -1676,12 +1684,13 @@ pfctl_show_limits(int dev, int opts)
 {
        unsigned int limit;
        int i;
+       int ret;
 
        if (opts & PF_OPT_SHOWALL)
                pfctl_print_title("LIMITS:");
        for (i = 0; pf_limits[i].name; i++) {
-               if (pfctl_get_limit(pfh, pf_limits[i].index, &limit))
-                       err(1, "DIOCGETLIMIT");
+               if ((ret = pfctl_get_limit(pfh, pf_limits[i].index, &limit)) != 
0)
+                       errc(1, ret, "DIOCGETLIMIT");
                printf("%-13s ", pf_limits[i].name);
                if (limit == UINT_MAX)
                        printf("unlimited\n");
@@ -1712,18 +1721,19 @@ int
 pfctl_add_pool(struct pfctl *pf, struct pfctl_pool *p, sa_family_t af)
 {
        struct pf_pooladdr *pa;
+       int ret;
 
        if ((pf->opts & PF_OPT_NOACTION) == 0) {
-               if (pfctl_begin_addrs(pf->h, &pf->paddr.ticket))
-                       err(1, "DIOCBEGINADDRS");
+               if ((ret = pfctl_begin_addrs(pf->h, &pf->paddr.ticket)) != 0)
+                       errc(1, ret, "DIOCBEGINADDRS");
        }
 
        pf->paddr.af = af;
        TAILQ_FOREACH(pa, &p->list, entries) {
                memcpy(&pf->paddr.addr, pa, sizeof(struct pf_pooladdr));
                if ((pf->opts & PF_OPT_NOACTION) == 0) {
-                       if (pfctl_add_addr(pf->h, &pf->paddr) != 0)
-                               err(1, "DIOCADDADDR");
+                       if ((ret = pfctl_add_addr(pf->h, &pf->paddr)) != 0)
+                               errc(1, ret, "DIOCADDADDR");
                }
        }
        return (0);
@@ -1932,6 +1942,7 @@ pfctl_load_eth_rule(struct pfctl *pf, char *path, struct 
pfctl_eth_rule *r,
        char                    *name;
        char                    anchor[PF_ANCHOR_NAME_SIZE];
        int                     len = strlen(path);
+       int                     ret;
 
        if (strlcpy(anchor, path, sizeof(anchor)) >= sizeof(anchor))
                errx(1, "pfctl_load_eth_rule: strlcpy");
@@ -1951,9 +1962,9 @@ pfctl_load_eth_rule(struct pfctl *pf, char *path, struct 
pfctl_eth_rule *r,
                name = "";
 
        if ((pf->opts & PF_OPT_NOACTION) == 0)
-               if (pfctl_add_eth_rule(pf->dev, r, anchor, name,
-                   pf->eth_ticket))
-                       err(1, "DIOCADDETHRULENV");
+               if ((ret = pfctl_add_eth_rule(pf->dev, r, anchor, name,
+                   pf->eth_ticket)) != 0)
+                       errc(1, ret, "DIOCADDETHRULENV");
 
        if (pf->opts & PF_OPT_VERBOSE) {
                INDENT(depth, !(pf->opts & PF_OPT_VERBOSE2));
@@ -2078,7 +2089,7 @@ pfctl_load_rule(struct pfctl *pf, char *path, struct 
pfctl_rule *r, int depth)
                        was_present = true;
                        break;
                default:
-                       err(1, "DIOCADDRULENV");
+                       errc(1, error, "DIOCADDRULE");
                }
        }
 
@@ -2679,6 +2690,7 @@ int
 pfctl_do_set_debug(struct pfctl *pf, char *d)
 {
        u_int32_t       level;
+       int             ret;
 
        if ((loadopt & PFCTL_FLAG_OPTION) == 0)
                return (0);
@@ -2700,8 +2712,8 @@ pfctl_do_set_debug(struct pfctl *pf, char *d)
        level = pf->debug;
 
        if ((pf->opts & PF_OPT_NOACTION) == 0)
-               if (pfctl_set_debug(pfh, level))
-                       err(1, "DIOCSETDEBUG");
+               if ((ret = pfctl_set_debug(pfh, level)) != 0)
+                       errc(1, ret, "DIOCSETDEBUG");
 
        if (pf->opts & PF_OPT_VERBOSE)
                printf("set debug %s\n", d);
@@ -2758,8 +2770,10 @@ pfctl_set_interface_flags(struct pfctl *pf, char 
*ifname, int flags, int how)
 void
 pfctl_debug(int dev, u_int32_t level, int opts)
 {
-       if (pfctl_set_debug(pfh, level))
-               err(1, "DIOCSETDEBUG");
+       int ret;
+
+       if ((ret = pfctl_set_debug(pfh, level)) != 0)
+               errc(1, ret, "DIOCSETDEBUG");
        if ((opts & PF_OPT_QUIET) == 0) {
                fprintf(stderr, "debug level set to '");
                switch (level) {
@@ -2852,15 +2866,15 @@ pfctl_show_eth_anchors(int dev, int opts, char 
*anchorname)
                        fprintf(stderr, "Anchor '%s' not found.\n",
                            anchorname);
                else
-                       err(1, "DIOCGETETHRULESETS");
+                       errc(1, ret, "DIOCGETETHRULESETS");
                return (-1);
        }
 
        for (int nr = 0; nr < ri.nr; nr++) {
                char sub[MAXPATHLEN];
 
-               if (pfctl_get_eth_ruleset(dev, anchorname, nr, &rs) != 0)
-                       err(1, "DIOCGETETHRULESET");
+               if ((ret = pfctl_get_eth_ruleset(dev, anchorname, nr, &rs)) != 
0)
+                       errc(1, ret, "DIOCGETETHRULESET");
 
                if (!strcmp(rs.name, PF_RESERVED_ANCHOR))
                        continue;

Reply via email to