From owner-dev-commits-src-branches@freebsd.org Tue May 11 15:06:51 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id AEFB363ABEC; Tue, 11 May 2021 15:06:51 +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 4Ffh8q2cRbz4T4L; Tue, 11 May 2021 15:06:50 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 6EAEC1A159; Tue, 11 May 2021 15:06:50 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 14BF6o0W049671; Tue, 11 May 2021 15:06:50 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 14BF6oHn049670; Tue, 11 May 2021 15:06:50 GMT (envelope-from git) Date: Tue, 11 May 2021 15:06:50 GMT Message-Id: <202105111506.14BF6oHn049670@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kristof Provost Subject: git: 09db9de2fea8 - stable/13 - pf: Error tracing SDTs 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/stable/13 X-Git-Reftype: branch X-Git-Commit: 09db9de2fea847de2ebc35acd7333c97390dc8f8 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 May 2021 15:06:51 -0000 The branch stable/13 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=09db9de2fea847de2ebc35acd7333c97390dc8f8 commit 09db9de2fea847de2ebc35acd7333c97390dc8f8 Author: Kristof Provost AuthorDate: 2021-04-20 09:18:26 +0000 Commit: Kristof Provost CommitDate: 2021-05-11 15:04:45 +0000 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") (cherry picked from commit 6b146f3b9b665c9baf6ba2cb038bbee359cb738a) --- 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 36e3fb8ff332..d9e313f62683 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; @@ -2492,7 +2492,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 @@ DIOCADDRULENV_error: 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 @@ DIOCGETSTATES_full: 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 @@ fail: 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 0a0f9beeef40..d50f46a1f5cd 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)