From 6b146f3b9b665c9baf6ba2cb038bbee359cb738a Mon Sep 17 00:00:00 2001 From: Kristof Provost Date: Tue, 20 Apr 2021 11:18:26 +0200 Subject: [PATCH] pf: Error tracing SDTs Add additional DTrace static trace points to facilitate debugging failing pf ioctl calls. MFC after: 1 week Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/netpfil/pf/pf_ioctl.c | 102 +++++++++++++++++++++----------------- sys/netpfil/pf/pf_nv.h | 1 + 2 files changed, 57 insertions(+), 46 deletions(-) diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index 8176ac82a52..4e4f726a561 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -63,6 +63,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -93,6 +94,12 @@ __FBSDID("$FreeBSD$"); #include #endif +SDT_PROVIDER_DECLARE(pf); +SDT_PROBE_DEFINE3(pf, ioctl, ioctl, error, "int", "int", "int"); +SDT_PROBE_DEFINE3(pf, ioctl, function, error, "char *", "int", "int"); +SDT_PROBE_DEFINE2(pf, ioctl, addrule, error, "int", "int"); +SDT_PROBE_DEFINE2(pf, ioctl, nvchk, error, "int", "int"); + static struct pf_kpool *pf_get_kpool(char *, u_int32_t, u_int8_t, u_int32_t, u_int8_t, u_int8_t, u_int8_t); @@ -260,6 +267,14 @@ pflog_packet_t *pflog_packet_ptr = NULL; extern u_long pf_ioctl_maxcount; +#define ERROUT_FUNCTION(target, x) \ + do { \ + error = (x); \ + SDT_PROBE3(pf, ioctl, function, error, __func__, error, \ + __LINE__); \ + goto target; \ + } while (0) + static void pfattach_vnet(void) { @@ -1962,23 +1977,23 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule) struct pf_krule *rule; int error = 0; +#define ERROUT(x) ERROUT_FUNCTION(errout, x) + rule = malloc(sizeof(*rule), M_PFRULE, M_WAITOK | M_ZERO); PFNV_CHK(pf_nvuint32(nvl, "nr", &rule->nr)); - if (! nvlist_exists_nvlist(nvl, "src")) { - error = EINVAL; - goto errout; - } + if (! nvlist_exists_nvlist(nvl, "src")) + ERROUT(EINVAL); + error = pf_nvrule_addr_to_rule_addr(nvlist_get_nvlist(nvl, "src"), &rule->src); if (error != 0) - goto errout; + ERROUT(error); + + if (! nvlist_exists_nvlist(nvl, "dst")) + ERROUT(EINVAL); - if (! nvlist_exists_nvlist(nvl, "dst")) { - error = EINVAL; - goto errout; - } PFNV_CHK(pf_nvrule_addr_to_rule_addr(nvlist_get_nvlist(nvl, "dst"), &rule->dst)); @@ -1991,18 +2006,14 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule) int ret; strs = nvlist_get_string_array(nvl, "labels", &items); - if (items > PF_RULE_MAX_LABEL_COUNT) { - error = E2BIG; - goto errout; - } + if (items > PF_RULE_MAX_LABEL_COUNT) + ERROUT(E2BIG); for (size_t i = 0; i < items; i++) { ret = strlcpy(rule->label[i], strs[i], sizeof(rule->label[0])); - if (ret >= sizeof(rule->label[0])) { - error = E2BIG; - goto errout; - } + if (ret >= sizeof(rule->label[0])) + ERROUT(E2BIG); } } @@ -2018,10 +2029,8 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule) PFNV_CHK(pf_nvstring(nvl, "overload_tblname", rule->overload_tblname, sizeof(rule->overload_tblname))); - if (! nvlist_exists_nvlist(nvl, "rpool")) { - error = EINVAL; - goto errout; - } + if (! nvlist_exists_nvlist(nvl, "rpool")) + ERROUT(EINVAL); PFNV_CHK(pf_nvpool_to_pool(nvlist_get_nvlist(nvl, "rpool"), &rule->rpool)); @@ -2047,17 +2056,13 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule) PFNV_CHK(pf_nvuint16(nvl, "max_mss", &rule->max_mss)); PFNV_CHK(pf_nvuint16(nvl, "scrub_flags", &rule->scrub_flags)); - if (! nvlist_exists_nvlist(nvl, "uid")) { - error = EINVAL; - goto errout; - } + if (! nvlist_exists_nvlist(nvl, "uid")) + ERROUT(EINVAL); PFNV_CHK(pf_nvrule_uid_to_rule_uid(nvlist_get_nvlist(nvl, "uid"), &rule->uid)); - if (! nvlist_exists_nvlist(nvl, "gid")) { - error = EINVAL; - goto errout; - } + if (! nvlist_exists_nvlist(nvl, "gid")) + ERROUT(EINVAL); PFNV_CHK(pf_nvrule_gid_to_rule_gid(nvlist_get_nvlist(nvl, "gid"), &rule->gid)); @@ -2095,10 +2100,8 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule) if (nvlist_exists_nvlist(nvl, "divert")) { const nvlist_t *nvldivert = nvlist_get_nvlist(nvl, "divert"); - if (! nvlist_exists_nvlist(nvldivert, "addr")) { - error = EINVAL; - goto errout; - } + if (! nvlist_exists_nvlist(nvldivert, "addr")) + ERROUT(EINVAL); PFNV_CHK(pf_nvaddr_to_addr(nvlist_get_nvlist(nvldivert, "addr"), &rule->divert.addr)); PFNV_CHK(pf_nvuint16(nvldivert, "port", &rule->divert.port)); @@ -2106,16 +2109,12 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule) /* Validation */ #ifndef INET - if (rule->af == AF_INET) { - error = EAFNOSUPPORT; - goto errout; - } + if (rule->af == AF_INET) + ERROUT(EAFNOSUPPORT); #endif /* INET */ #ifndef INET6 - if (rule->af == AF_INET6) { - error = EAFNOSUPPORT; - goto errout; - } + if (rule->af == AF_INET6) + ERROUT(EAFNOSUPPORT); #endif /* INET6 */ PFNV_CHK(pf_check_rule_addr(&rule->src)); @@ -2125,6 +2124,7 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_krule **prule) return (0); +#undef ERROUT errout: pf_krule_free(rule); *prule = NULL; @@ -2412,7 +2412,7 @@ pf_ioctl_addrule(struct pf_krule *rule, uint32_t ticket, goto errout_unlocked; } -#define ERROUT(x) { error = (x); goto errout; } +#define ERROUT(x) ERROUT_FUNCTION(errout, x) if (rule->ifname[0]) kif = pf_kkif_create(M_WAITOK); @@ -2639,6 +2639,14 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td int error = 0; PF_RULES_RLOCK_TRACKER; +#define ERROUT_IOCTL(target, x) \ + do { \ + error = (x); \ + SDT_PROBE3(pf, ioctl, ioctl, error, cmd, error, __LINE__); \ + goto target; \ + } while (0) + + /* XXX keep in sync with switch() below */ if (securelevel_gt(td->td_ucred, 2)) switch (cmd) { @@ -2793,7 +2801,7 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td const char *anchor = "", *anchor_call = ""; uint32_t ticket = 0, pool_ticket = 0; -#define ERROUT(x) do { error = (x); goto DIOCADDRULENV_error; } while (0) +#define ERROUT(x) ERROUT_IOCTL(DIOCADDRULENV_error, x) if (nv->len > pf_ioctl_maxcount) ERROUT(ENOMEM); @@ -2962,7 +2970,7 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td int rs_num, nr; bool clear_counter = false; -#define ERROUT(x) do { error = (x); goto DIOCGETRULENV_error; } while (0) +#define ERROUT(x) ERROUT_IOCTL(DIOCGETRULENV_error, x) if (nv->len > pf_ioctl_maxcount) ERROUT(ENOMEM); @@ -3997,7 +4005,7 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td kif = pf_kkif_create(M_WAITOK); newpa->kif = NULL; } -#define ERROUT(x) { error = (x); goto DIOCCHANGEADDR_error; } +#define ERROUT(x) ERROUT_IOCTL(DIOCCHANGEADDR_error, x) PF_RULES_WLOCK(); ruleset = pf_find_kruleset(pca->anchor); if (ruleset == NULL) @@ -5129,6 +5137,8 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td sx_xunlock(&pf_ioctl_lock); CURVNET_RESTORE(); +#undef ERROUT_IOCTL + return (error); } @@ -5337,7 +5347,7 @@ pf_keepcounters(struct pfioc_nv *nv) void *nvlpacked = NULL; int error = 0; -#define ERROUT(x) do { error = (x); goto on_error; } while (0) +#define ERROUT(x) ERROUT_FUNCTION(on_error, x) if (nv->len > pf_ioctl_maxcount) ERROUT(ENOMEM); diff --git a/sys/netpfil/pf/pf_nv.h b/sys/netpfil/pf/pf_nv.h index 0a0f9beeef4..d50f46a1f5c 100644 --- a/sys/netpfil/pf/pf_nv.h +++ b/sys/netpfil/pf/pf_nv.h @@ -53,6 +53,7 @@ int pf_nvstring(const nvlist_t *, const char *, char *, size_t); #define PFNV_CHK(x) do { \ error = (x); \ + SDT_PROBE2(pf, ioctl, nvchk, error, error, __LINE__); \ if (error != 0) \ goto errout; \ } while (0) -- 2.45.0