All of lore.kernel.org
 help / color / mirror / Atom feed
* netfilter: x_tables: ratelimit most printks
@ 2018-02-07 13:48 Florian Westphal
  2018-02-07 13:48 ` [PATCH nf 1/7] netfilter: x_tables: remove pr_info where possible Florian Westphal
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 13:48 UTC (permalink / raw)
  To: netfilter-devel

Aeons ago, before namespaces, there was no need to ratelimit this:
all of these error messages got triggered in response to iptables
commands, which need CAP_NET_ADMIN.

Nowadays we have namespaces, so its better to ratelimit these.
This should also help fuzzing (syzkaller), as it can generate a large
volume of error messages (which are useless there).

The patches are split as follows:
- first get rid of printks that should never be triggered, as userland
  doesn't generate such malformed rules anyway.
- second, switch some printks to pr_debug.  This is mostly for messages
  where it might make sense for developers to see what exactly went
  wrong.

Rest of the patches swap remaining pr_foo with pr_foo_ratelimited().

Note that most patches introduce overly long lines, but splitting these
would make it necessary to split the error messages which is worse.

46 files changed, 254 insertions(+), 257 deletions(-)


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH nf 1/7] netfilter: x_tables: remove pr_info where possible
  2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
@ 2018-02-07 13:48 ` Florian Westphal
  2018-02-07 17:03   ` Pablo Neira Ayuso
  2018-02-07 13:48 ` [PATCH nf 2/7] netfilter: x_tables: prefer pr_debug " Florian Westphal
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 13:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

remove several pr_info messages that cannot be triggered with iptables.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/netfilter/ipt_ECN.c | 10 ++++------
 net/netfilter/xt_HL.c        | 13 +++----------
 net/netfilter/xt_LED.c       |  4 +---
 net/netfilter/xt_cgroup.c    |  4 +---
 4 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_ECN.c b/net/ipv4/netfilter/ipt_ECN.c
index 270765236f5e..39ff167e6d86 100644
--- a/net/ipv4/netfilter/ipt_ECN.c
+++ b/net/ipv4/netfilter/ipt_ECN.c
@@ -98,14 +98,12 @@ static int ecn_tg_check(const struct xt_tgchk_param *par)
 	const struct ipt_ECN_info *einfo = par->targinfo;
 	const struct ipt_entry *e = par->entryinfo;
 
-	if (einfo->operation & IPT_ECN_OP_MASK) {
-		pr_info("unsupported ECN operation %x\n", einfo->operation);
+	if (einfo->operation & IPT_ECN_OP_MASK)
 		return -EINVAL;
-	}
-	if (einfo->ip_ect & ~IPT_ECN_IP_MASK) {
-		pr_info("new ECT codepoint %x out of mask\n", einfo->ip_ect);
+
+	if (einfo->ip_ect & ~IPT_ECN_IP_MASK)
 		return -EINVAL;
-	}
+
 	if ((einfo->operation & (IPT_ECN_OP_SET_ECE|IPT_ECN_OP_SET_CWR)) &&
 	    (e->ip.proto != IPPROTO_TCP || (e->ip.invflags & XT_INV_PROTO))) {
 		pr_info("cannot use TCP operations on a non-tcp rule\n");
diff --git a/net/netfilter/xt_HL.c b/net/netfilter/xt_HL.c
index 1535e87ed9bd..4653b071bed4 100644
--- a/net/netfilter/xt_HL.c
+++ b/net/netfilter/xt_HL.c
@@ -105,10 +105,8 @@ static int ttl_tg_check(const struct xt_tgchk_param *par)
 {
 	const struct ipt_TTL_info *info = par->targinfo;
 
-	if (info->mode > IPT_TTL_MAXMODE) {
-		pr_info("TTL: invalid or unknown mode %u\n", info->mode);
+	if (info->mode > IPT_TTL_MAXMODE)
 		return -EINVAL;
-	}
 	if (info->mode != IPT_TTL_SET && info->ttl == 0)
 		return -EINVAL;
 	return 0;
@@ -118,15 +116,10 @@ static int hl_tg6_check(const struct xt_tgchk_param *par)
 {
 	const struct ip6t_HL_info *info = par->targinfo;
 
-	if (info->mode > IP6T_HL_MAXMODE) {
-		pr_info("invalid or unknown mode %u\n", info->mode);
+	if (info->mode > IP6T_HL_MAXMODE)
 		return -EINVAL;
-	}
-	if (info->mode != IP6T_HL_SET && info->hop_limit == 0) {
-		pr_info("increment/decrement does not "
-			"make sense with value 0\n");
+	if (info->mode != IP6T_HL_SET && info->hop_limit == 0)
 		return -EINVAL;
-	}
 	return 0;
 }
 
diff --git a/net/netfilter/xt_LED.c b/net/netfilter/xt_LED.c
index 1dcad893df78..ece311c11fdc 100644
--- a/net/netfilter/xt_LED.c
+++ b/net/netfilter/xt_LED.c
@@ -111,10 +111,8 @@ static int led_tg_check(const struct xt_tgchk_param *par)
 	struct xt_led_info_internal *ledinternal;
 	int err;
 
-	if (ledinfo->id[0] == '\0') {
-		pr_info("No 'id' parameter given.\n");
+	if (ledinfo->id[0] == '\0')
 		return -EINVAL;
-	}
 
 	mutex_lock(&xt_led_mutex);
 
diff --git a/net/netfilter/xt_cgroup.c b/net/netfilter/xt_cgroup.c
index 891f4e7e8ea7..556530db7dbb 100644
--- a/net/netfilter/xt_cgroup.c
+++ b/net/netfilter/xt_cgroup.c
@@ -42,10 +42,8 @@ static int cgroup_mt_check_v1(const struct xt_mtchk_param *par)
 	if ((info->invert_path & ~1) || (info->invert_classid & ~1))
 		return -EINVAL;
 
-	if (!info->has_path && !info->has_classid) {
-		pr_info("xt_cgroup: no path or classid specified\n");
+	if (!info->has_path && !info->has_classid)
 		return -EINVAL;
-	}
 
 	if (info->has_path && info->has_classid) {
 		pr_info("xt_cgroup: both path and classid specified\n");
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH nf 2/7] netfilter: x_tables: prefer pr_debug where possible
  2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
  2018-02-07 13:48 ` [PATCH nf 1/7] netfilter: x_tables: remove pr_info where possible Florian Westphal
@ 2018-02-07 13:48 ` Florian Westphal
  2018-02-07 17:02   ` Pablo Neira Ayuso
  2018-02-07 13:48 ` [PATCH nf 3/7] netfilter: xt_CT: use pr ratelimiting Florian Westphal
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 13:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

prefer pr_debug for cases where error is usually not seen by users.
checkpatch complains due to lines > 80 but adding a newline doesn't
make things any more readable.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/netfilter/ipt_rpfilter.c  | 2 +-
 net/ipv6/netfilter/ip6t_rpfilter.c | 2 +-
 net/netfilter/xt_SECMARK.c         | 2 +-
 net/netfilter/xt_bpf.c             | 2 +-
 net/netfilter/xt_connlabel.c       | 2 +-
 net/netfilter/xt_ipcomp.c          | 2 +-
 net/netfilter/xt_ipvs.c            | 2 +-
 net/netfilter/xt_l2tp.c            | 2 +-
 net/netfilter/xt_recent.c          | 4 ++--
 net/netfilter/xt_socket.c          | 8 ++++----
 net/netfilter/xt_time.c            | 2 +-
 11 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
index 37fb9552e858..ffd1cf65af3a 100644
--- a/net/ipv4/netfilter/ipt_rpfilter.c
+++ b/net/ipv4/netfilter/ipt_rpfilter.c
@@ -105,7 +105,7 @@ static int rpfilter_check(const struct xt_mtchk_param *par)
 	const struct xt_rpfilter_info *info = par->matchinfo;
 	unsigned int options = ~XT_RPFILTER_OPTION_MASK;
 	if (info->flags & options) {
-		pr_info("unknown options encountered");
+		pr_debug("unknown options");
 		return -EINVAL;
 	}
 
diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
index b12e61b7b16c..c9e27d4687a2 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -103,7 +103,7 @@ static int rpfilter_check(const struct xt_mtchk_param *par)
 	unsigned int options = ~XT_RPFILTER_OPTION_MASK;
 
 	if (info->flags & options) {
-		pr_info("unknown options encountered");
+		pr_debug("unknown options");
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index 9faf5e050b79..36f7ad881a7e 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -101,7 +101,7 @@ static int secmark_tg_check(const struct xt_tgchk_param *par)
 	case SECMARK_MODE_SEL:
 		break;
 	default:
-		pr_info("invalid mode: %hu\n", info->mode);
+		pr_debug("invalid mode: %hu\n", info->mode);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_bpf.c b/net/netfilter/xt_bpf.c
index 06b090d8e901..77a12ef9e11e 100644
--- a/net/netfilter/xt_bpf.c
+++ b/net/netfilter/xt_bpf.c
@@ -34,7 +34,7 @@ static int __bpf_mt_check_bytecode(struct sock_filter *insns, __u16 len,
 	program.filter = insns;
 
 	if (bpf_prog_create(ret, &program)) {
-		pr_info("bpf: check failed: parse error\n");
+		pr_debug("bpf: check failed: parse error\n");
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_connlabel.c b/net/netfilter/xt_connlabel.c
index 23372879e6e3..cf3031e4ff61 100644
--- a/net/netfilter/xt_connlabel.c
+++ b/net/netfilter/xt_connlabel.c
@@ -57,7 +57,7 @@ static int connlabel_mt_check(const struct xt_mtchk_param *par)
 	int ret;
 
 	if (info->options & ~options) {
-		pr_err("Unknown options in mask %x\n", info->options);
+		pr_debug("Unknown options in mask %x\n", info->options);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_ipcomp.c b/net/netfilter/xt_ipcomp.c
index 7ca64a50db04..1ecde0efe879 100644
--- a/net/netfilter/xt_ipcomp.c
+++ b/net/netfilter/xt_ipcomp.c
@@ -72,7 +72,7 @@ static int comp_mt_check(const struct xt_mtchk_param *par)
 
 	/* Must specify no unknown invflags */
 	if (compinfo->invflags & ~XT_IPCOMP_INV_MASK) {
-		pr_err("unknown flags %X\n", compinfo->invflags);
+		pr_debug("unknown flags %X\n", compinfo->invflags);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/netfilter/xt_ipvs.c b/net/netfilter/xt_ipvs.c
index 42540d26c2b8..e5ffc2f1424c 100644
--- a/net/netfilter/xt_ipvs.c
+++ b/net/netfilter/xt_ipvs.c
@@ -158,7 +158,7 @@ static int ipvs_mt_check(const struct xt_mtchk_param *par)
 	    && par->family != NFPROTO_IPV6
 #endif
 		) {
-		pr_info("protocol family %u not supported\n", par->family);
+		pr_debug("protocol family %u not supported\n", par->family);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_l2tp.c b/net/netfilter/xt_l2tp.c
index 8aee572771f2..54ac58b309e5 100644
--- a/net/netfilter/xt_l2tp.c
+++ b/net/netfilter/xt_l2tp.c
@@ -216,7 +216,7 @@ static int l2tp_mt_check(const struct xt_mtchk_param *par)
 	/* Check for invalid flags */
 	if (info->flags & ~(XT_L2TP_TID | XT_L2TP_SID | XT_L2TP_VERSION |
 			    XT_L2TP_TYPE)) {
-		pr_info("unknown flags: %x\n", info->flags);
+		pr_debug("unknown flags: %x\n", info->flags);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
index 245fa350a7a8..db6a2d43bb30 100644
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -342,8 +342,8 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
 	net_get_random_once(&hash_rnd, sizeof(hash_rnd));
 
 	if (info->check_set & ~XT_RECENT_VALID_FLAGS) {
-		pr_info("Unsupported user space flags (%08x)\n",
-			info->check_set);
+		pr_debug("Unsupported userspace flags (%08x)\n",
+			 info->check_set);
 		return -EINVAL;
 	}
 	if (hweight8(info->check_set &
diff --git a/net/netfilter/xt_socket.c b/net/netfilter/xt_socket.c
index 575d2153e3b8..5a0b16bc29c8 100644
--- a/net/netfilter/xt_socket.c
+++ b/net/netfilter/xt_socket.c
@@ -171,7 +171,7 @@ static int socket_mt_v1_check(const struct xt_mtchk_param *par)
 		return err;
 
 	if (info->flags & ~XT_SOCKET_FLAGS_V1) {
-		pr_info("unknown flags 0x%x\n", info->flags & ~XT_SOCKET_FLAGS_V1);
+		pr_debug("unknown flags 0x%x\n", info->flags & ~XT_SOCKET_FLAGS_V1);
 		return -EINVAL;
 	}
 	return 0;
@@ -187,7 +187,7 @@ static int socket_mt_v2_check(const struct xt_mtchk_param *par)
 		return err;
 
 	if (info->flags & ~XT_SOCKET_FLAGS_V2) {
-		pr_info("unknown flags 0x%x\n", info->flags & ~XT_SOCKET_FLAGS_V2);
+		pr_debug("unknown flags 0x%x\n", info->flags & ~XT_SOCKET_FLAGS_V2);
 		return -EINVAL;
 	}
 	return 0;
@@ -203,8 +203,8 @@ static int socket_mt_v3_check(const struct xt_mtchk_param *par)
 	if (err)
 		return err;
 	if (info->flags & ~XT_SOCKET_FLAGS_V3) {
-		pr_info("unknown flags 0x%x\n",
-			info->flags & ~XT_SOCKET_FLAGS_V3);
+		pr_debug("unknown flags 0x%x\n",
+			 info->flags & ~XT_SOCKET_FLAGS_V3);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
index 1b01eec1fbda..aea2b5a12ed7 100644
--- a/net/netfilter/xt_time.c
+++ b/net/netfilter/xt_time.c
@@ -241,7 +241,7 @@ static int time_mt_check(const struct xt_mtchk_param *par)
 	}
 
 	if (info->flags & ~XT_TIME_ALL_FLAGS) {
-		pr_info("unknown flags 0x%x\n", info->flags & ~XT_TIME_ALL_FLAGS);
+		pr_debug("unknown flags 0x%x\n", info->flags & ~XT_TIME_ALL_FLAGS);
 		return -EINVAL;
 	}
 
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH nf 3/7] netfilter: xt_CT: use pr ratelimiting
  2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
  2018-02-07 13:48 ` [PATCH nf 1/7] netfilter: x_tables: remove pr_info where possible Florian Westphal
  2018-02-07 13:48 ` [PATCH nf 2/7] netfilter: x_tables: prefer pr_debug " Florian Westphal
@ 2018-02-07 13:48 ` Florian Westphal
  2018-02-07 13:48 ` [PATCH nf 4/7] netfilter: x_tables: rate limit pr_err warnings Florian Westphal
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 13:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

checkpatch complains about line > 80 but this would require splitting
"literal" over two lines which is worse.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/xt_CT.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index 5a152e2acfd5..8790190c6feb 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -82,15 +82,14 @@ xt_ct_set_helper(struct nf_conn *ct, const char *helper_name,
 
 	proto = xt_ct_find_proto(par);
 	if (!proto) {
-		pr_info("You must specify a L4 protocol, and not use "
-			"inversions on it.\n");
+		pr_info_ratelimited("You must specify a L4 protocol and not use inversions on it\n");
 		return -ENOENT;
 	}
 
 	helper = nf_conntrack_helper_try_module_get(helper_name, par->family,
 						    proto);
 	if (helper == NULL) {
-		pr_info("No such helper \"%s\"\n", helper_name);
+		pr_info_ratelimited("No such helper \"%s\"\n", helper_name);
 		return -ENOENT;
 	}
 
@@ -124,6 +123,7 @@ xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
 	const struct nf_conntrack_l4proto *l4proto;
 	struct ctnl_timeout *timeout;
 	struct nf_conn_timeout *timeout_ext;
+	const char *errmsg = NULL;
 	int ret = 0;
 	u8 proto;
 
@@ -131,29 +131,29 @@ xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
 	timeout_find_get = rcu_dereference(nf_ct_timeout_find_get_hook);
 	if (timeout_find_get == NULL) {
 		ret = -ENOENT;
-		pr_info("Timeout policy base is empty\n");
+		errmsg = "Timeout policy base is empty";
 		goto out;
 	}
 
 	proto = xt_ct_find_proto(par);
 	if (!proto) {
 		ret = -EINVAL;
-		pr_info("You must specify a L4 protocol, and not use "
-			"inversions on it.\n");
+		errmsg = "You must specify a L4 protocol and not use inversions on it";
 		goto out;
 	}
 
 	timeout = timeout_find_get(par->net, timeout_name);
 	if (timeout == NULL) {
 		ret = -ENOENT;
-		pr_info("No such timeout policy \"%s\"\n", timeout_name);
+		pr_info_ratelimited("No such timeout policy \"%s\"\n",
+				    timeout_name);
 		goto out;
 	}
 
 	if (timeout->l3num != par->family) {
 		ret = -EINVAL;
-		pr_info("Timeout policy `%s' can only be used by L3 protocol "
-			"number %d\n", timeout_name, timeout->l3num);
+		pr_info_ratelimited("Timeout policy `%s' can only be used by L%d protocol number %d\n",
+				    timeout_name, 3, timeout->l3num);
 		goto err_put_timeout;
 	}
 	/* Make sure the timeout policy matches any existing protocol tracker,
@@ -162,9 +162,8 @@ xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
 	l4proto = __nf_ct_l4proto_find(par->family, proto);
 	if (timeout->l4proto->l4proto != l4proto->l4proto) {
 		ret = -EINVAL;
-		pr_info("Timeout policy `%s' can only be used by L4 protocol "
-			"number %d\n",
-			timeout_name, timeout->l4proto->l4proto);
+		pr_info_ratelimited("Timeout policy `%s' can only be used by L%d protocol number %d\n",
+				    timeout_name, 4, timeout->l4proto->l4proto);
 		goto err_put_timeout;
 	}
 	timeout_ext = nf_ct_timeout_ext_add(ct, timeout, GFP_ATOMIC);
@@ -180,6 +179,8 @@ xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
 	__xt_ct_tg_timeout_put(timeout);
 out:
 	rcu_read_unlock();
+	if (errmsg)
+		pr_info_ratelimited("%s\n", errmsg);
 	return ret;
 #else
 	return -EOPNOTSUPP;
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH nf 4/7] netfilter: x_tables: rate limit pr_err warnings
  2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
                   ` (2 preceding siblings ...)
  2018-02-07 13:48 ` [PATCH nf 3/7] netfilter: xt_CT: use pr ratelimiting Florian Westphal
@ 2018-02-07 13:48 ` Florian Westphal
  2018-02-07 13:48 ` [PATCH nf 5/7] netfilter: x_tables: rate-limit table mismatch warnings Florian Westphal
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 13:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/x_tables.c | 70 +++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 36 deletions(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 2f685ee1f9c8..0f81294dea7b 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -434,36 +434,35 @@ int xt_check_match(struct xt_mtchk_param *par,
 		 * ebt_among is exempt from centralized matchsize checking
 		 * because it uses a dynamic-size data set.
 		 */
-		pr_err("%s_tables: %s.%u match: invalid size "
-		       "%u (kernel) != (user) %u\n",
-		       xt_prefix[par->family], par->match->name,
-		       par->match->revision,
-		       XT_ALIGN(par->match->matchsize), size);
+		pr_err_ratelimited("%s_tables: %s.%u match: invalid size %u (kernel) != (user) %u\n",
+				   xt_prefix[par->family], par->match->name,
+				   par->match->revision,
+				   XT_ALIGN(par->match->matchsize), size);
 		return -EINVAL;
 	}
 	if (par->match->table != NULL &&
 	    strcmp(par->match->table, par->table) != 0) {
-		pr_err("%s_tables: %s match: only valid in %s table, not %s\n",
-		       xt_prefix[par->family], par->match->name,
-		       par->match->table, par->table);
+		pr_err_ratelimited("%s_tables: %s match: only valid in %s table, not %s\n",
+				   xt_prefix[par->family], par->match->name,
+				   par->match->table, par->table);
 		return -EINVAL;
 	}
 	if (par->match->hooks && (par->hook_mask & ~par->match->hooks) != 0) {
 		char used[64], allow[64];
 
-		pr_err("%s_tables: %s match: used from hooks %s, but only "
-		       "valid from %s\n",
-		       xt_prefix[par->family], par->match->name,
-		       textify_hooks(used, sizeof(used), par->hook_mask,
-		                     par->family),
-		       textify_hooks(allow, sizeof(allow), par->match->hooks,
-		                     par->family));
+		pr_err_ratelimited("%s_tables: %s match: used from hooks %s, but only valid from %s\n",
+				   xt_prefix[par->family], par->match->name,
+				   textify_hooks(used, sizeof(used),
+						 par->hook_mask, par->family),
+				   textify_hooks(allow, sizeof(allow),
+						 par->match->hooks,
+						 par->family));
 		return -EINVAL;
 	}
 	if (par->match->proto && (par->match->proto != proto || inv_proto)) {
-		pr_err("%s_tables: %s match: only valid for protocol %u\n",
-		       xt_prefix[par->family], par->match->name,
-		       par->match->proto);
+		pr_err_ratelimited("%s_tables: %s match: only valid for protocol %u\n",
+				   xt_prefix[par->family], par->match->name,
+				   par->match->proto);
 		return -EINVAL;
 	}
 	if (par->match->checkentry != NULL) {
@@ -814,36 +813,35 @@ int xt_check_target(struct xt_tgchk_param *par,
 	int ret;
 
 	if (XT_ALIGN(par->target->targetsize) != size) {
-		pr_err("%s_tables: %s.%u target: invalid size "
-		       "%u (kernel) != (user) %u\n",
-		       xt_prefix[par->family], par->target->name,
-		       par->target->revision,
-		       XT_ALIGN(par->target->targetsize), size);
+		pr_err_ratelimited("%s_tables: %s.%u target: invalid size %u (kernel) != (user) %u\n",
+				   xt_prefix[par->family], par->target->name,
+				   par->target->revision,
+				   XT_ALIGN(par->target->targetsize), size);
 		return -EINVAL;
 	}
 	if (par->target->table != NULL &&
 	    strcmp(par->target->table, par->table) != 0) {
-		pr_err("%s_tables: %s target: only valid in %s table, not %s\n",
-		       xt_prefix[par->family], par->target->name,
-		       par->target->table, par->table);
+		pr_err_ratelimited("%s_tables: %s target: only valid in %s table, not %s\n",
+				   xt_prefix[par->family], par->target->name,
+				   par->target->table, par->table);
 		return -EINVAL;
 	}
 	if (par->target->hooks && (par->hook_mask & ~par->target->hooks) != 0) {
 		char used[64], allow[64];
 
-		pr_err("%s_tables: %s target: used from hooks %s, but only "
-		       "usable from %s\n",
-		       xt_prefix[par->family], par->target->name,
-		       textify_hooks(used, sizeof(used), par->hook_mask,
-		                     par->family),
-		       textify_hooks(allow, sizeof(allow), par->target->hooks,
-		                     par->family));
+		pr_err_ratelimited("%s_tables: %s target: used from hooks %s, but only usable from %s\n",
+				   xt_prefix[par->family], par->target->name,
+				   textify_hooks(used, sizeof(used),
+						 par->hook_mask, par->family),
+				   textify_hooks(allow, sizeof(allow),
+						 par->target->hooks,
+						 par->family));
 		return -EINVAL;
 	}
 	if (par->target->proto && (par->target->proto != proto || inv_proto)) {
-		pr_err("%s_tables: %s target: only valid for protocol %u\n",
-		       xt_prefix[par->family], par->target->name,
-		       par->target->proto);
+		pr_err_ratelimited("%s_tables: %s target: only valid for protocol %u\n",
+				   xt_prefix[par->family], par->target->name,
+				   par->target->proto);
 		return -EINVAL;
 	}
 	if (par->target->checkentry != NULL) {
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH nf 5/7] netfilter: x_tables: rate-limit table mismatch warnings
  2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
                   ` (3 preceding siblings ...)
  2018-02-07 13:48 ` [PATCH nf 4/7] netfilter: x_tables: rate limit pr_err warnings Florian Westphal
@ 2018-02-07 13:48 ` Florian Westphal
  2018-02-07 13:48 ` [PATCH nf 6/7] netfilter: x_tables: use pr ratelimiting Florian Westphal
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 13:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/netfilter/ipt_rpfilter.c  | 4 ++--
 net/ipv6/netfilter/ip6t_rpfilter.c | 4 ++--
 net/netfilter/xt_CONNSECMARK.c     | 4 ++--
 net/netfilter/xt_SECMARK.c         | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
index ffd1cf65af3a..afea7aff80c1 100644
--- a/net/ipv4/netfilter/ipt_rpfilter.c
+++ b/net/ipv4/netfilter/ipt_rpfilter.c
@@ -111,8 +111,8 @@ static int rpfilter_check(const struct xt_mtchk_param *par)
 
 	if (strcmp(par->table, "mangle") != 0 &&
 	    strcmp(par->table, "raw") != 0) {
-		pr_info("match only valid in the \'raw\' "
-			"or \'mangle\' tables, not \'%s\'.\n", par->table);
+		pr_info_ratelimited("only valid in \'raw\' or \'mangle\' table, not \'%s\'\n",
+				    par->table);
 		return -EINVAL;
 	}
 
diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
index c9e27d4687a2..9740f9c6428c 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -109,8 +109,8 @@ static int rpfilter_check(const struct xt_mtchk_param *par)
 
 	if (strcmp(par->table, "mangle") != 0 &&
 	    strcmp(par->table, "raw") != 0) {
-		pr_info("match only valid in the \'raw\' "
-			"or \'mangle\' tables, not \'%s\'.\n", par->table);
+		pr_info_ratelimited("only valid in \'raw\' or \'mangle\' table, not \'%s\'\n",
+				    par->table);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_CONNSECMARK.c b/net/netfilter/xt_CONNSECMARK.c
index da56c06a443c..6f30cd399e42 100644
--- a/net/netfilter/xt_CONNSECMARK.c
+++ b/net/netfilter/xt_CONNSECMARK.c
@@ -91,8 +91,8 @@ static int connsecmark_tg_check(const struct xt_tgchk_param *par)
 
 	if (strcmp(par->table, "mangle") != 0 &&
 	    strcmp(par->table, "security") != 0) {
-		pr_info("target only valid in the \'mangle\' "
-			"or \'security\' tables, not \'%s\'.\n", par->table);
+		pr_info_ratelimited("only valid in \'mangle\' or \'security\' table, not \'%s\'\n",
+				    par->table);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index 36f7ad881a7e..1f2a0627478a 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -86,8 +86,8 @@ static int secmark_tg_check(const struct xt_tgchk_param *par)
 
 	if (strcmp(par->table, "mangle") != 0 &&
 	    strcmp(par->table, "security") != 0) {
-		pr_info("target only valid in the \'mangle\' "
-			"or \'security\' tables, not \'%s\'.\n", par->table);
+		pr_info_ratelimited("only valid in \'mangle\' or \'security\' table, not \'%s\'\n",
+				    par->table);
 		return -EINVAL;
 	}
 
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH nf 6/7] netfilter: x_tables: use pr ratelimiting
  2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
                   ` (4 preceding siblings ...)
  2018-02-07 13:48 ` [PATCH nf 5/7] netfilter: x_tables: rate-limit table mismatch warnings Florian Westphal
@ 2018-02-07 13:48 ` Florian Westphal
  2018-02-07 13:48 ` [PATCH nf 7/7] netfilter: x_tables: use pr ratelimiting in all remaining spots Florian Westphal
  2018-02-14 19:49 ` netfilter: x_tables: ratelimit most printks Pablo Neira Ayuso
  7 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 13:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

all of these print simple error message - use single pr_ratelimit call.
checkpatch complains about lines > 80 but this would require splitting
several "literals" over multiple lines which is worse.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/xt_HMARK.c    | 24 ++++++++++++++----------
 net/netfilter/xt_addrtype.c | 33 ++++++++++++++++-----------------
 net/netfilter/xt_policy.c   | 23 +++++++++++++----------
 3 files changed, 43 insertions(+), 37 deletions(-)

diff --git a/net/netfilter/xt_HMARK.c b/net/netfilter/xt_HMARK.c
index 60e6dbe12460..3345bed8b200 100644
--- a/net/netfilter/xt_HMARK.c
+++ b/net/netfilter/xt_HMARK.c
@@ -9,6 +9,8 @@
  * the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/icmp.h>
@@ -312,29 +314,31 @@ hmark_tg_v4(struct sk_buff *skb, const struct xt_action_param *par)
 static int hmark_tg_check(const struct xt_tgchk_param *par)
 {
 	const struct xt_hmark_info *info = par->targinfo;
+	const char *errmsg = "hash modulus can't be zero";
 
-	if (!info->hmodulus) {
-		pr_info("xt_HMARK: hash modulus can't be zero\n");
-		return -EINVAL;
-	}
+	if (!info->hmodulus)
+		goto err;
 	if (info->proto_mask &&
 	    (info->flags & XT_HMARK_FLAG(XT_HMARK_METHOD_L3))) {
-		pr_info("xt_HMARK: proto mask must be zero with L3 mode\n");
-		return -EINVAL;
+		errmsg = "proto mask must be zero with L3 mode";
+		goto err;
 	}
 	if (info->flags & XT_HMARK_FLAG(XT_HMARK_SPI_MASK) &&
 	    (info->flags & (XT_HMARK_FLAG(XT_HMARK_SPORT_MASK) |
 			     XT_HMARK_FLAG(XT_HMARK_DPORT_MASK)))) {
-		pr_info("xt_HMARK: spi-mask and port-mask can't be combined\n");
-		return -EINVAL;
+		errmsg = "spi-mask and port-mask can't be combined";
+		goto err;
 	}
 	if (info->flags & XT_HMARK_FLAG(XT_HMARK_SPI) &&
 	    (info->flags & (XT_HMARK_FLAG(XT_HMARK_SPORT) |
 			     XT_HMARK_FLAG(XT_HMARK_DPORT)))) {
-		pr_info("xt_HMARK: spi-set and port-set can't be combined\n");
-		return -EINVAL;
+		errmsg = "spi-set and port-set can't be combined";
+		goto err;
 	}
 	return 0;
+err:
+	pr_info_ratelimited("%s\n", errmsg);
+	return -EINVAL;
 }
 
 static struct xt_target hmark_tg_reg[] __read_mostly = {
diff --git a/net/netfilter/xt_addrtype.c b/net/netfilter/xt_addrtype.c
index 911a7c0da504..89e281b3bfc2 100644
--- a/net/netfilter/xt_addrtype.c
+++ b/net/netfilter/xt_addrtype.c
@@ -164,48 +164,47 @@ addrtype_mt_v1(const struct sk_buff *skb, struct xt_action_param *par)
 
 static int addrtype_mt_checkentry_v1(const struct xt_mtchk_param *par)
 {
+	const char *errmsg = "both incoming and outgoing interface limitation cannot be selected";
 	struct xt_addrtype_info_v1 *info = par->matchinfo;
 
 	if (info->flags & XT_ADDRTYPE_LIMIT_IFACE_IN &&
-	    info->flags & XT_ADDRTYPE_LIMIT_IFACE_OUT) {
-		pr_info("both incoming and outgoing "
-			"interface limitation cannot be selected\n");
-		return -EINVAL;
-	}
+	    info->flags & XT_ADDRTYPE_LIMIT_IFACE_OUT)
+		goto err;
 
 	if (par->hook_mask & ((1 << NF_INET_PRE_ROUTING) |
 	    (1 << NF_INET_LOCAL_IN)) &&
 	    info->flags & XT_ADDRTYPE_LIMIT_IFACE_OUT) {
-		pr_info("output interface limitation "
-			"not valid in PREROUTING and INPUT\n");
-		return -EINVAL;
+		errmsg = "output interface limitation not valid in PREROUTING and INPUT";
+		goto err;
 	}
 
 	if (par->hook_mask & ((1 << NF_INET_POST_ROUTING) |
 	    (1 << NF_INET_LOCAL_OUT)) &&
 	    info->flags & XT_ADDRTYPE_LIMIT_IFACE_IN) {
-		pr_info("input interface limitation "
-			"not valid in POSTROUTING and OUTPUT\n");
-		return -EINVAL;
+		errmsg = "input interface limitation not valid in POSTROUTING and OUTPUT";
+		goto err;
 	}
 
 #if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
 	if (par->family == NFPROTO_IPV6) {
 		if ((info->source | info->dest) & XT_ADDRTYPE_BLACKHOLE) {
-			pr_err("ipv6 BLACKHOLE matching not supported\n");
-			return -EINVAL;
+			errmsg = "ipv6 BLACKHOLE matching not supported";
+			goto err;
 		}
 		if ((info->source | info->dest) >= XT_ADDRTYPE_PROHIBIT) {
-			pr_err("ipv6 PROHIBIT (THROW, NAT ..) matching not supported\n");
-			return -EINVAL;
+			errmsg = "ipv6 PROHIBIT (THROW, NAT ..) matching not supported";
+			goto err;
 		}
 		if ((info->source | info->dest) & XT_ADDRTYPE_BROADCAST) {
-			pr_err("ipv6 does not support BROADCAST matching\n");
-			return -EINVAL;
+			errmsg = "ipv6 does not support BROADCAST matching";
+			goto err;
 		}
 	}
 #endif
 	return 0;
+err:
+	pr_info_ratelimited("%s\n", errmsg);
+	return -EINVAL;
 }
 
 static struct xt_match addrtype_mt_reg[] __read_mostly = {
diff --git a/net/netfilter/xt_policy.c b/net/netfilter/xt_policy.c
index 5639fb03bdd9..13f8ccf946d6 100644
--- a/net/netfilter/xt_policy.c
+++ b/net/netfilter/xt_policy.c
@@ -132,26 +132,29 @@ policy_mt(const struct sk_buff *skb, struct xt_action_param *par)
 static int policy_mt_check(const struct xt_mtchk_param *par)
 {
 	const struct xt_policy_info *info = par->matchinfo;
+	const char *errmsg = "neither incoming nor outgoing policy selected";
+
+	if (!(info->flags & (XT_POLICY_MATCH_IN|XT_POLICY_MATCH_OUT)))
+		goto err;
 
-	if (!(info->flags & (XT_POLICY_MATCH_IN|XT_POLICY_MATCH_OUT))) {
-		pr_info("neither incoming nor outgoing policy selected\n");
-		return -EINVAL;
-	}
 	if (par->hook_mask & ((1 << NF_INET_PRE_ROUTING) |
 	    (1 << NF_INET_LOCAL_IN)) && info->flags & XT_POLICY_MATCH_OUT) {
-		pr_info("output policy not valid in PREROUTING and INPUT\n");
-		return -EINVAL;
+		errmsg = "output policy not valid in PREROUTING and INPUT";
+		goto err;
 	}
 	if (par->hook_mask & ((1 << NF_INET_POST_ROUTING) |
 	    (1 << NF_INET_LOCAL_OUT)) && info->flags & XT_POLICY_MATCH_IN) {
-		pr_info("input policy not valid in POSTROUTING and OUTPUT\n");
-		return -EINVAL;
+		errmsg = "input policy not valid in POSTROUTING and OUTPUT";
+		goto err;
 	}
 	if (info->len > XT_POLICY_MAX_ELEM) {
-		pr_info("too many policy elements\n");
-		return -EINVAL;
+		errmsg = "too many policy elements";
+		goto err;
 	}
 	return 0;
+err:
+	pr_info_ratelimited("%s\n", errmsg);
+	return -EINVAL;
 }
 
 static struct xt_match policy_mt_reg[] __read_mostly = {
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH nf 7/7] netfilter: x_tables: use pr ratelimiting in all remaining spots
  2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
                   ` (5 preceding siblings ...)
  2018-02-07 13:48 ` [PATCH nf 6/7] netfilter: x_tables: use pr ratelimiting Florian Westphal
@ 2018-02-07 13:48 ` Florian Westphal
  2018-02-07 17:00   ` Pablo Neira Ayuso
  2018-02-14 19:49 ` netfilter: x_tables: ratelimit most printks Pablo Neira Ayuso
  7 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 13:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/bridge/netfilter/ebt_among.c | 10 ++++----
 net/bridge/netfilter/ebt_limit.c |  4 ++--
 net/ipv4/netfilter/ipt_ECN.c     |  2 +-
 net/ipv4/netfilter/ipt_REJECT.c  |  4 ++--
 net/ipv6/netfilter/ip6t_REJECT.c |  4 ++--
 net/ipv6/netfilter/ip6t_srh.c    |  6 +++--
 net/netfilter/xt_AUDIT.c         |  4 ++--
 net/netfilter/xt_CHECKSUM.c      |  5 ++--
 net/netfilter/xt_CONNSECMARK.c   |  6 ++---
 net/netfilter/xt_DSCP.c          |  2 +-
 net/netfilter/xt_LED.c           |  2 +-
 net/netfilter/xt_NFQUEUE.c       |  6 ++---
 net/netfilter/xt_SECMARK.c       | 12 ++++++----
 net/netfilter/xt_TCPMSS.c        | 10 ++++----
 net/netfilter/xt_TPROXY.c        |  6 ++---
 net/netfilter/xt_cgroup.c        |  8 ++++---
 net/netfilter/xt_cluster.c       |  8 +++----
 net/netfilter/xt_connbytes.c     |  4 ++--
 net/netfilter/xt_connlabel.c     |  4 ++--
 net/netfilter/xt_connmark.c      |  8 +++----
 net/netfilter/xt_conntrack.c     |  4 ++--
 net/netfilter/xt_dscp.c          |  2 +-
 net/netfilter/xt_ecn.c           |  4 ++--
 net/netfilter/xt_hashlimit.c     | 24 ++++++++++---------
 net/netfilter/xt_helper.c        |  4 ++--
 net/netfilter/xt_l2tp.c          | 20 +++++++++-------
 net/netfilter/xt_limit.c         |  4 ++--
 net/netfilter/xt_nat.c           |  5 ++--
 net/netfilter/xt_nfacct.c        |  6 +++--
 net/netfilter/xt_physdev.c       |  4 +---
 net/netfilter/xt_recent.c        | 10 ++++----
 net/netfilter/xt_set.c           | 50 ++++++++++++++++++++--------------------
 net/netfilter/xt_state.c         |  4 ++--
 net/netfilter/xt_time.c          |  3 +--
 34 files changed, 132 insertions(+), 127 deletions(-)

diff --git a/net/bridge/netfilter/ebt_among.c b/net/bridge/netfilter/ebt_among.c
index 279527f8b1fe..12d850a3ea68 100644
--- a/net/bridge/netfilter/ebt_among.c
+++ b/net/bridge/netfilter/ebt_among.c
@@ -187,17 +187,17 @@ static int ebt_among_mt_check(const struct xt_mtchk_param *par)
 	expected_length += ebt_mac_wormhash_size(wh_src);
 
 	if (em->match_size != EBT_ALIGN(expected_length)) {
-		pr_info("wrong size: %d against expected %d, rounded to %zd\n",
-			em->match_size, expected_length,
-			EBT_ALIGN(expected_length));
+		pr_info_ratelimited("wrong size: %d against expected %d, rounded to %zd\n",
+				    em->match_size, expected_length,
+				    EBT_ALIGN(expected_length));
 		return -EINVAL;
 	}
 	if (wh_dst && (err = ebt_mac_wormhash_check_integrity(wh_dst))) {
-		pr_info("dst integrity fail: %x\n", -err);
+		pr_info_ratelimited("dst integrity fail: %x\n", -err);
 		return -EINVAL;
 	}
 	if (wh_src && (err = ebt_mac_wormhash_check_integrity(wh_src))) {
-		pr_info("src integrity fail: %x\n", -err);
+		pr_info_ratelimited("src integrity fail: %x\n", -err);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/bridge/netfilter/ebt_limit.c b/net/bridge/netfilter/ebt_limit.c
index 61a9f1be1263..165b9d678cf1 100644
--- a/net/bridge/netfilter/ebt_limit.c
+++ b/net/bridge/netfilter/ebt_limit.c
@@ -72,8 +72,8 @@ static int ebt_limit_mt_check(const struct xt_mtchk_param *par)
 	/* Check for overflow. */
 	if (info->burst == 0 ||
 	    user2credits(info->avg * info->burst) < user2credits(info->avg)) {
-		pr_info("overflow, try lower: %u/%u\n",
-			info->avg, info->burst);
+		pr_info_ratelimited("overflow, try lower: %u/%u\n",
+				    info->avg, info->burst);
 		return -EINVAL;
 	}
 
diff --git a/net/ipv4/netfilter/ipt_ECN.c b/net/ipv4/netfilter/ipt_ECN.c
index 39ff167e6d86..aaaf9a81fbc9 100644
--- a/net/ipv4/netfilter/ipt_ECN.c
+++ b/net/ipv4/netfilter/ipt_ECN.c
@@ -106,7 +106,7 @@ static int ecn_tg_check(const struct xt_tgchk_param *par)
 
 	if ((einfo->operation & (IPT_ECN_OP_SET_ECE|IPT_ECN_OP_SET_CWR)) &&
 	    (e->ip.proto != IPPROTO_TCP || (e->ip.invflags & XT_INV_PROTO))) {
-		pr_info("cannot use TCP operations on a non-tcp rule\n");
+		pr_info_ratelimited("cannot use operation on non-tcp rule\n");
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/ipv4/netfilter/ipt_REJECT.c b/net/ipv4/netfilter/ipt_REJECT.c
index 8bd0d7b26632..e8bed3390e58 100644
--- a/net/ipv4/netfilter/ipt_REJECT.c
+++ b/net/ipv4/netfilter/ipt_REJECT.c
@@ -74,13 +74,13 @@ static int reject_tg_check(const struct xt_tgchk_param *par)
 	const struct ipt_entry *e = par->entryinfo;
 
 	if (rejinfo->with == IPT_ICMP_ECHOREPLY) {
-		pr_info("ECHOREPLY no longer supported.\n");
+		pr_info_ratelimited("ECHOREPLY no longer supported.\n");
 		return -EINVAL;
 	} else if (rejinfo->with == IPT_TCP_RESET) {
 		/* Must specify that it's a TCP packet */
 		if (e->ip.proto != IPPROTO_TCP ||
 		    (e->ip.invflags & XT_INV_PROTO)) {
-			pr_info("TCP_RESET invalid for non-tcp\n");
+			pr_info_ratelimited("TCP_RESET invalid for non-tcp\n");
 			return -EINVAL;
 		}
 	}
diff --git a/net/ipv6/netfilter/ip6t_REJECT.c b/net/ipv6/netfilter/ip6t_REJECT.c
index fa51a205918d..38dea8ff680f 100644
--- a/net/ipv6/netfilter/ip6t_REJECT.c
+++ b/net/ipv6/netfilter/ip6t_REJECT.c
@@ -85,14 +85,14 @@ static int reject_tg6_check(const struct xt_tgchk_param *par)
 	const struct ip6t_entry *e = par->entryinfo;
 
 	if (rejinfo->with == IP6T_ICMP6_ECHOREPLY) {
-		pr_info("ECHOREPLY is not supported.\n");
+		pr_info_ratelimited("ECHOREPLY is not supported\n");
 		return -EINVAL;
 	} else if (rejinfo->with == IP6T_TCP_RESET) {
 		/* Must specify that it's a TCP packet */
 		if (!(e->ipv6.flags & IP6T_F_PROTO) ||
 		    e->ipv6.proto != IPPROTO_TCP ||
 		    (e->ipv6.invflags & XT_INV_PROTO)) {
-			pr_info("TCP_RESET illegal for non-tcp\n");
+			pr_info_ratelimited("TCP_RESET illegal for non-tcp\n");
 			return -EINVAL;
 		}
 	}
diff --git a/net/ipv6/netfilter/ip6t_srh.c b/net/ipv6/netfilter/ip6t_srh.c
index 9642164107ce..580dffa68e60 100644
--- a/net/ipv6/netfilter/ip6t_srh.c
+++ b/net/ipv6/netfilter/ip6t_srh.c
@@ -122,12 +122,14 @@ static int srh_mt6_check(const struct xt_mtchk_param *par)
 	const struct ip6t_srh *srhinfo = par->matchinfo;
 
 	if (srhinfo->mt_flags & ~IP6T_SRH_MASK) {
-		pr_err("unknown srh match flags  %X\n", srhinfo->mt_flags);
+		pr_err_ratelimited("unknown srh match flags  %X\n",
+				   srhinfo->mt_flags);
 		return -EINVAL;
 	}
 
 	if (srhinfo->mt_invflags & ~IP6T_SRH_INV_MASK) {
-		pr_err("unknown srh invflags %X\n", srhinfo->mt_invflags);
+		pr_err_ratelimited("unknown srh invflags %X\n",
+				   srhinfo->mt_invflags);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
index c502419d6306..f368ee6741db 100644
--- a/net/netfilter/xt_AUDIT.c
+++ b/net/netfilter/xt_AUDIT.c
@@ -120,8 +120,8 @@ static int audit_tg_check(const struct xt_tgchk_param *par)
 	const struct xt_audit_info *info = par->targinfo;
 
 	if (info->type > XT_AUDIT_TYPE_MAX) {
-		pr_info("Audit type out of range (valid range: 0..%hhu)\n",
-			XT_AUDIT_TYPE_MAX);
+		pr_info_ratelimited("Audit type out of range (valid range: 0..%hhu)\n",
+				    XT_AUDIT_TYPE_MAX);
 		return -ERANGE;
 	}
 
diff --git a/net/netfilter/xt_CHECKSUM.c b/net/netfilter/xt_CHECKSUM.c
index 0f642ef8cd26..a6b00f2f6f49 100644
--- a/net/netfilter/xt_CHECKSUM.c
+++ b/net/netfilter/xt_CHECKSUM.c
@@ -36,11 +36,12 @@ static int checksum_tg_check(const struct xt_tgchk_param *par)
 	const struct xt_CHECKSUM_info *einfo = par->targinfo;
 
 	if (einfo->operation & ~XT_CHECKSUM_OP_FILL) {
-		pr_info("unsupported CHECKSUM operation %x\n", einfo->operation);
+		pr_info_ratelimited("unsupported CHECKSUM operation %x\n",
+				    einfo->operation);
 		return -EINVAL;
 	}
 	if (!einfo->operation) {
-		pr_info("no CHECKSUM operation enabled\n");
+		pr_info_ratelimited("no CHECKSUM operation enabled\n");
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/netfilter/xt_CONNSECMARK.c b/net/netfilter/xt_CONNSECMARK.c
index 6f30cd399e42..f3f1caac949b 100644
--- a/net/netfilter/xt_CONNSECMARK.c
+++ b/net/netfilter/xt_CONNSECMARK.c
@@ -102,14 +102,14 @@ static int connsecmark_tg_check(const struct xt_tgchk_param *par)
 		break;
 
 	default:
-		pr_info("invalid mode: %hu\n", info->mode);
+		pr_info_ratelimited("invalid mode: %hu\n", info->mode);
 		return -EINVAL;
 	}
 
 	ret = nf_ct_netns_get(par->net, par->family);
 	if (ret < 0)
-		pr_info("cannot load conntrack support for proto=%u\n",
-			par->family);
+		pr_info_ratelimited("cannot load conntrack support for proto=%u\n",
+				    par->family);
 	return ret;
 }
 
diff --git a/net/netfilter/xt_DSCP.c b/net/netfilter/xt_DSCP.c
index 3f83d38c4e5b..503a84401788 100644
--- a/net/netfilter/xt_DSCP.c
+++ b/net/netfilter/xt_DSCP.c
@@ -67,7 +67,7 @@ static int dscp_tg_check(const struct xt_tgchk_param *par)
 	const struct xt_DSCP_info *info = par->targinfo;
 
 	if (info->dscp > XT_DSCP_MAX) {
-		pr_info("dscp %x out of range\n", info->dscp);
+		pr_info_ratelimited("dscp %x out of range\n", info->dscp);
 		return -EDOM;
 	}
 	return 0;
diff --git a/net/netfilter/xt_LED.c b/net/netfilter/xt_LED.c
index ece311c11fdc..5eeb3cfdae22 100644
--- a/net/netfilter/xt_LED.c
+++ b/net/netfilter/xt_LED.c
@@ -136,7 +136,7 @@ static int led_tg_check(const struct xt_tgchk_param *par)
 
 	err = led_trigger_register(&ledinternal->netfilter_led_trigger);
 	if (err) {
-		pr_err("Trigger name is already in use.\n");
+		pr_err_ratelimited("Trigger name is already in use.\n");
 		goto exit_alloc;
 	}
 
diff --git a/net/netfilter/xt_NFQUEUE.c b/net/netfilter/xt_NFQUEUE.c
index a360b99a958a..9fac4710f7cf 100644
--- a/net/netfilter/xt_NFQUEUE.c
+++ b/net/netfilter/xt_NFQUEUE.c
@@ -67,13 +67,13 @@ static int nfqueue_tg_check(const struct xt_tgchk_param *par)
 	init_hashrandom(&jhash_initval);
 
 	if (info->queues_total == 0) {
-		pr_err("NFQUEUE: number of total queues is 0\n");
+		pr_err_ratelimited("NFQUEUE: number of total queues is 0\n");
 		return -EINVAL;
 	}
 	maxid = info->queues_total - 1 + info->queuenum;
 	if (maxid > 0xffff) {
-		pr_err("NFQUEUE: number of queues (%u) out of range (got %u)\n",
-		       info->queues_total, maxid);
+		pr_err_ratelimited("NFQUEUE: number of queues (%u) out of range (got %u)\n",
+				   info->queues_total, maxid);
 		return -ERANGE;
 	}
 	if (par->target->revision == 2 && info->flags > 1)
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index 1f2a0627478a..852004ef33c5 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -60,18 +60,20 @@ static int checkentry_lsm(struct xt_secmark_target_info *info)
 				       &info->secid);
 	if (err) {
 		if (err == -EINVAL)
-			pr_info("invalid security context \'%s\'\n", info->secctx);
+			pr_info_ratelimited("invalid security context \'%s\'\n",
+					    info->secctx);
 		return err;
 	}
 
 	if (!info->secid) {
-		pr_info("unable to map security context \'%s\'\n", info->secctx);
+		pr_info_ratelimited("unable to map security context \'%s\'\n",
+				    info->secctx);
 		return -ENOENT;
 	}
 
 	err = security_secmark_relabel_packet(info->secid);
 	if (err) {
-		pr_info("unable to obtain relabeling permission\n");
+		pr_info_ratelimited("unable to obtain relabeling permission\n");
 		return err;
 	}
 
@@ -92,8 +94,8 @@ static int secmark_tg_check(const struct xt_tgchk_param *par)
 	}
 
 	if (mode && mode != info->mode) {
-		pr_info("mode already set to %hu cannot mix with "
-			"rules for mode %hu\n", mode, info->mode);
+		pr_info_ratelimited("mode already set to %hu cannot mix with rules for mode %hu\n",
+				    mode, info->mode);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 99bb8e410f22..98efb202f8b4 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -273,8 +273,7 @@ static int tcpmss_tg4_check(const struct xt_tgchk_param *par)
 	    (par->hook_mask & ~((1 << NF_INET_FORWARD) |
 			   (1 << NF_INET_LOCAL_OUT) |
 			   (1 << NF_INET_POST_ROUTING))) != 0) {
-		pr_info("path-MTU clamping only supported in "
-			"FORWARD, OUTPUT and POSTROUTING hooks\n");
+		pr_info_ratelimited("path-MTU clamping only supported in FORWARD, OUTPUT and POSTROUTING hooks\n");
 		return -EINVAL;
 	}
 	if (par->nft_compat)
@@ -283,7 +282,7 @@ static int tcpmss_tg4_check(const struct xt_tgchk_param *par)
 	xt_ematch_foreach(ematch, e)
 		if (find_syn_match(ematch))
 			return 0;
-	pr_info("Only works on TCP SYN packets\n");
+	pr_info_ratelimited("Only works on TCP SYN packets\n");
 	return -EINVAL;
 }
 
@@ -298,8 +297,7 @@ static int tcpmss_tg6_check(const struct xt_tgchk_param *par)
 	    (par->hook_mask & ~((1 << NF_INET_FORWARD) |
 			   (1 << NF_INET_LOCAL_OUT) |
 			   (1 << NF_INET_POST_ROUTING))) != 0) {
-		pr_info("path-MTU clamping only supported in "
-			"FORWARD, OUTPUT and POSTROUTING hooks\n");
+		pr_info_ratelimited("path-MTU clamping only supported in FORWARD, OUTPUT and POSTROUTING hooks\n");
 		return -EINVAL;
 	}
 	if (par->nft_compat)
@@ -308,7 +306,7 @@ static int tcpmss_tg6_check(const struct xt_tgchk_param *par)
 	xt_ematch_foreach(ematch, e)
 		if (find_syn_match(ematch))
 			return 0;
-	pr_info("Only works on TCP SYN packets\n");
+	pr_info_ratelimited("Only works on TCP SYN packets\n");
 	return -EINVAL;
 }
 #endif
diff --git a/net/netfilter/xt_TPROXY.c b/net/netfilter/xt_TPROXY.c
index 17d7705e3bd4..8c89323c06af 100644
--- a/net/netfilter/xt_TPROXY.c
+++ b/net/netfilter/xt_TPROXY.c
@@ -540,8 +540,7 @@ static int tproxy_tg6_check(const struct xt_tgchk_param *par)
 	    !(i->invflags & IP6T_INV_PROTO))
 		return 0;
 
-	pr_info("Can be used only in combination with "
-		"either -p tcp or -p udp\n");
+	pr_info_ratelimited("Can be used only with -p tcp or -p udp\n");
 	return -EINVAL;
 }
 #endif
@@ -559,8 +558,7 @@ static int tproxy_tg4_check(const struct xt_tgchk_param *par)
 	    && !(i->invflags & IPT_INV_PROTO))
 		return 0;
 
-	pr_info("Can be used only in combination with "
-		"either -p tcp or -p udp\n");
+	pr_info_ratelimited("Can be used only with -p tcp or -p udp\n");
 	return -EINVAL;
 }
 
diff --git a/net/netfilter/xt_cgroup.c b/net/netfilter/xt_cgroup.c
index 556530db7dbb..6f8c4077a07f 100644
--- a/net/netfilter/xt_cgroup.c
+++ b/net/netfilter/xt_cgroup.c
@@ -12,6 +12,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/skbuff.h>
 #include <linux/module.h>
 #include <linux/netfilter/x_tables.h>
@@ -46,7 +48,7 @@ static int cgroup_mt_check_v1(const struct xt_mtchk_param *par)
 		return -EINVAL;
 
 	if (info->has_path && info->has_classid) {
-		pr_info("xt_cgroup: both path and classid specified\n");
+		pr_info_ratelimited("path and classid specified\n");
 		return -EINVAL;
 	}
 
@@ -54,8 +56,8 @@ static int cgroup_mt_check_v1(const struct xt_mtchk_param *par)
 	if (info->has_path) {
 		cgrp = cgroup_get_from_path(info->path);
 		if (IS_ERR(cgrp)) {
-			pr_info("xt_cgroup: invalid path, errno=%ld\n",
-				PTR_ERR(cgrp));
+			pr_info_ratelimited("invalid path, errno=%ld\n",
+					    PTR_ERR(cgrp));
 			return -EINVAL;
 		}
 		info->priv = cgrp;
diff --git a/net/netfilter/xt_cluster.c b/net/netfilter/xt_cluster.c
index 57ef175dfbfa..0068688995c8 100644
--- a/net/netfilter/xt_cluster.c
+++ b/net/netfilter/xt_cluster.c
@@ -135,14 +135,12 @@ static int xt_cluster_mt_checkentry(const struct xt_mtchk_param *par)
 	struct xt_cluster_match_info *info = par->matchinfo;
 
 	if (info->total_nodes > XT_CLUSTER_NODES_MAX) {
-		pr_info("you have exceeded the maximum "
-			"number of cluster nodes (%u > %u)\n",
-			info->total_nodes, XT_CLUSTER_NODES_MAX);
+		pr_info_ratelimited("you have exceeded the maximum number of cluster nodes (%u > %u)\n",
+				    info->total_nodes, XT_CLUSTER_NODES_MAX);
 		return -EINVAL;
 	}
 	if (info->node_mask >= (1ULL << info->total_nodes)) {
-		pr_info("this node mask cannot be "
-			"higher than the total number of nodes\n");
+		pr_info_ratelimited("node mask cannot exceed total number of nodes\n");
 		return -EDOM;
 	}
 	return 0;
diff --git a/net/netfilter/xt_connbytes.c b/net/netfilter/xt_connbytes.c
index cad0b7b5eb35..93cb018c3055 100644
--- a/net/netfilter/xt_connbytes.c
+++ b/net/netfilter/xt_connbytes.c
@@ -112,8 +112,8 @@ static int connbytes_mt_check(const struct xt_mtchk_param *par)
 
 	ret = nf_ct_netns_get(par->net, par->family);
 	if (ret < 0)
-		pr_info("cannot load conntrack support for proto=%u\n",
-			par->family);
+		pr_info_ratelimited("cannot load conntrack support for proto=%u\n",
+				    par->family);
 
 	/*
 	 * This filter cannot function correctly unless connection tracking
diff --git a/net/netfilter/xt_connlabel.c b/net/netfilter/xt_connlabel.c
index cf3031e4ff61..78954047a6de 100644
--- a/net/netfilter/xt_connlabel.c
+++ b/net/netfilter/xt_connlabel.c
@@ -63,8 +63,8 @@ static int connlabel_mt_check(const struct xt_mtchk_param *par)
 
 	ret = nf_ct_netns_get(par->net, par->family);
 	if (ret < 0) {
-		pr_info("cannot load conntrack support for proto=%u\n",
-							par->family);
+		pr_info_ratelimited("cannot load conntrack support for proto=%u\n",
+				    par->family);
 		return ret;
 	}
 
diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c
index ec377cc6a369..809639ce6f5a 100644
--- a/net/netfilter/xt_connmark.c
+++ b/net/netfilter/xt_connmark.c
@@ -79,8 +79,8 @@ static int connmark_tg_check(const struct xt_tgchk_param *par)
 
 	ret = nf_ct_netns_get(par->net, par->family);
 	if (ret < 0)
-		pr_info("cannot load conntrack support for proto=%u\n",
-			par->family);
+		pr_info_ratelimited("cannot load conntrack support for proto=%u\n",
+				    par->family);
 	return ret;
 }
 
@@ -109,8 +109,8 @@ static int connmark_mt_check(const struct xt_mtchk_param *par)
 
 	ret = nf_ct_netns_get(par->net, par->family);
 	if (ret < 0)
-		pr_info("cannot load conntrack support for proto=%u\n",
-			par->family);
+		pr_info_ratelimited("cannot load conntrack support for proto=%u\n",
+				    par->family);
 	return ret;
 }
 
diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
index 39cf1d019240..df80fe7d391c 100644
--- a/net/netfilter/xt_conntrack.c
+++ b/net/netfilter/xt_conntrack.c
@@ -272,8 +272,8 @@ static int conntrack_mt_check(const struct xt_mtchk_param *par)
 
 	ret = nf_ct_netns_get(par->net, par->family);
 	if (ret < 0)
-		pr_info("cannot load conntrack support for proto=%u\n",
-			par->family);
+		pr_info_ratelimited("cannot load conntrack support for proto=%u\n",
+				    par->family);
 	return ret;
 }
 
diff --git a/net/netfilter/xt_dscp.c b/net/netfilter/xt_dscp.c
index 236ac8008909..3d9a49516316 100644
--- a/net/netfilter/xt_dscp.c
+++ b/net/netfilter/xt_dscp.c
@@ -47,7 +47,7 @@ static int dscp_mt_check(const struct xt_mtchk_param *par)
 	const struct xt_dscp_info *info = par->matchinfo;
 
 	if (info->dscp > XT_DSCP_MAX) {
-		pr_info("dscp %x out of range\n", info->dscp);
+		pr_info_ratelimited("dscp %x out of range\n", info->dscp);
 		return -EDOM;
 	}
 
diff --git a/net/netfilter/xt_ecn.c b/net/netfilter/xt_ecn.c
index 3c831a8efebc..c7ad4afa5fb8 100644
--- a/net/netfilter/xt_ecn.c
+++ b/net/netfilter/xt_ecn.c
@@ -97,7 +97,7 @@ static int ecn_mt_check4(const struct xt_mtchk_param *par)
 
 	if (info->operation & (XT_ECN_OP_MATCH_ECE | XT_ECN_OP_MATCH_CWR) &&
 	    (ip->proto != IPPROTO_TCP || ip->invflags & IPT_INV_PROTO)) {
-		pr_info("cannot match TCP bits in rule for non-tcp packets\n");
+		pr_info_ratelimited("cannot match TCP bits for non-tcp packets\n");
 		return -EINVAL;
 	}
 
@@ -139,7 +139,7 @@ static int ecn_mt_check6(const struct xt_mtchk_param *par)
 
 	if (info->operation & (XT_ECN_OP_MATCH_ECE | XT_ECN_OP_MATCH_CWR) &&
 	    (ip->proto != IPPROTO_TCP || ip->invflags & IP6T_INV_PROTO)) {
-		pr_info("cannot match TCP bits in rule for non-tcp packets\n");
+		pr_info_ratelimited("cannot match TCP bits for non-tcp packets\n");
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index ca6847403ca2..9d8355920965 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -523,7 +523,8 @@ static u64 user2rate(u64 user)
 	if (user != 0) {
 		return div64_u64(XT_HASHLIMIT_SCALE_v2, user);
 	} else {
-		pr_warn("invalid rate from userspace: %llu\n", user);
+		pr_warn_ratelimited("invalid rate from userspace: %llu\n",
+				    user);
 		return 0;
 	}
 }
@@ -865,33 +866,34 @@ static int hashlimit_mt_check_common(const struct xt_mtchk_param *par,
 	}
 
 	if (cfg->mode & ~XT_HASHLIMIT_ALL) {
-		pr_info("Unknown mode mask %X, kernel too old?\n",
-						cfg->mode);
+		pr_info_ratelimited("Unknown mode mask %X, kernel too old?\n",
+				    cfg->mode);
 		return -EINVAL;
 	}
 
 	/* Check for overflow. */
 	if (revision >= 3 && cfg->mode & XT_HASHLIMIT_RATE_MATCH) {
 		if (cfg->avg == 0 || cfg->avg > U32_MAX) {
-			pr_info("hashlimit invalid rate\n");
+			pr_info_ratelimited("invalid rate\n");
 			return -ERANGE;
 		}
 
 		if (cfg->interval == 0) {
-			pr_info("hashlimit invalid interval\n");
+			pr_info_ratelimited("invalid interval\n");
 			return -EINVAL;
 		}
 	} else if (cfg->mode & XT_HASHLIMIT_BYTES) {
 		if (user2credits_byte(cfg->avg) == 0) {
-			pr_info("overflow, rate too high: %llu\n", cfg->avg);
+			pr_info_ratelimited("overflow, rate too high: %llu\n",
+					    cfg->avg);
 			return -EINVAL;
 		}
 	} else if (cfg->burst == 0 ||
-		    user2credits(cfg->avg * cfg->burst, revision) <
-		    user2credits(cfg->avg, revision)) {
-			pr_info("overflow, try lower: %llu/%llu\n",
-				cfg->avg, cfg->burst);
-			return -ERANGE;
+		   user2credits(cfg->avg * cfg->burst, revision) <
+		   user2credits(cfg->avg, revision)) {
+		pr_info_ratelimited("overflow, try lower: %llu/%llu\n",
+				    cfg->avg, cfg->burst);
+		return -ERANGE;
 	}
 
 	mutex_lock(&hashlimit_mutex);
diff --git a/net/netfilter/xt_helper.c b/net/netfilter/xt_helper.c
index 38a78151c0e9..fd077aeaaed9 100644
--- a/net/netfilter/xt_helper.c
+++ b/net/netfilter/xt_helper.c
@@ -61,8 +61,8 @@ static int helper_mt_check(const struct xt_mtchk_param *par)
 
 	ret = nf_ct_netns_get(par->net, par->family);
 	if (ret < 0) {
-		pr_info("cannot load conntrack support for proto=%u\n",
-			par->family);
+		pr_info_ratelimited("cannot load conntrack support for proto=%u\n",
+				    par->family);
 		return ret;
 	}
 	info->name[sizeof(info->name) - 1] = '\0';
diff --git a/net/netfilter/xt_l2tp.c b/net/netfilter/xt_l2tp.c
index 54ac58b309e5..8fcd20b32012 100644
--- a/net/netfilter/xt_l2tp.c
+++ b/net/netfilter/xt_l2tp.c
@@ -225,7 +225,8 @@ static int l2tp_mt_check(const struct xt_mtchk_param *par)
 	    (!(info->flags & XT_L2TP_SID)) &&
 	    ((!(info->flags & XT_L2TP_TYPE)) ||
 	     (info->type != XT_L2TP_TYPE_CONTROL))) {
-		pr_info("invalid flags combination: %x\n", info->flags);
+		pr_info_ratelimited("invalid flags combination: %x\n",
+				    info->flags);
 		return -EINVAL;
 	}
 
@@ -234,19 +235,22 @@ static int l2tp_mt_check(const struct xt_mtchk_param *par)
 	 */
 	if (info->flags & XT_L2TP_VERSION) {
 		if ((info->version < 2) || (info->version > 3)) {
-			pr_info("wrong L2TP version: %u\n", info->version);
+			pr_info_ratelimited("wrong L2TP version: %u\n",
+					    info->version);
 			return -EINVAL;
 		}
 
 		if (info->version == 2) {
 			if ((info->flags & XT_L2TP_TID) &&
 			    (info->tid > 0xffff)) {
-				pr_info("v2 tid > 0xffff: %u\n", info->tid);
+				pr_info_ratelimited("v2 tid > 0xffff: %u\n",
+						    info->tid);
 				return -EINVAL;
 			}
 			if ((info->flags & XT_L2TP_SID) &&
 			    (info->sid > 0xffff)) {
-				pr_info("v2 sid > 0xffff: %u\n", info->sid);
+				pr_info_ratelimited("v2 sid > 0xffff: %u\n",
+						    info->sid);
 				return -EINVAL;
 			}
 		}
@@ -268,13 +272,13 @@ static int l2tp_mt_check4(const struct xt_mtchk_param *par)
 
 	if ((ip->proto != IPPROTO_UDP) &&
 	    (ip->proto != IPPROTO_L2TP)) {
-		pr_info("missing protocol rule (udp|l2tpip)\n");
+		pr_info_ratelimited("missing protocol rule (udp|l2tpip)\n");
 		return -EINVAL;
 	}
 
 	if ((ip->proto == IPPROTO_L2TP) &&
 	    (info->version == 2)) {
-		pr_info("v2 doesn't support IP mode\n");
+		pr_info_ratelimited("v2 doesn't support IP mode\n");
 		return -EINVAL;
 	}
 
@@ -295,13 +299,13 @@ static int l2tp_mt_check6(const struct xt_mtchk_param *par)
 
 	if ((ip->proto != IPPROTO_UDP) &&
 	    (ip->proto != IPPROTO_L2TP)) {
-		pr_info("missing protocol rule (udp|l2tpip)\n");
+		pr_info_ratelimited("missing protocol rule (udp|l2tpip)\n");
 		return -EINVAL;
 	}
 
 	if ((ip->proto == IPPROTO_L2TP) &&
 	    (info->version == 2)) {
-		pr_info("v2 doesn't support IP mode\n");
+		pr_info_ratelimited("v2 doesn't support IP mode\n");
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_limit.c b/net/netfilter/xt_limit.c
index 61403b77361c..55d18cd67635 100644
--- a/net/netfilter/xt_limit.c
+++ b/net/netfilter/xt_limit.c
@@ -106,8 +106,8 @@ static int limit_mt_check(const struct xt_mtchk_param *par)
 	/* Check for overflow. */
 	if (r->burst == 0
 	    || user2credits(r->avg * r->burst) < user2credits(r->avg)) {
-		pr_info("Overflow, try lower: %u/%u\n",
-			r->avg, r->burst);
+		pr_info_ratelimited("Overflow, try lower: %u/%u\n",
+				    r->avg, r->burst);
 		return -ERANGE;
 	}
 
diff --git a/net/netfilter/xt_nat.c b/net/netfilter/xt_nat.c
index 0fd14d1eb09d..bdb689cdc829 100644
--- a/net/netfilter/xt_nat.c
+++ b/net/netfilter/xt_nat.c
@@ -8,6 +8,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/netfilter.h>
@@ -19,8 +21,7 @@ static int xt_nat_checkentry_v0(const struct xt_tgchk_param *par)
 	const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
 
 	if (mr->rangesize != 1) {
-		pr_info("%s: multiple ranges no longer supported\n",
-			par->target->name);
+		pr_info_ratelimited("multiple ranges no longer supported\n");
 		return -EINVAL;
 	}
 	return nf_ct_netns_get(par->net, par->family);
diff --git a/net/netfilter/xt_nfacct.c b/net/netfilter/xt_nfacct.c
index 6f92d25590a8..c8674deed4eb 100644
--- a/net/netfilter/xt_nfacct.c
+++ b/net/netfilter/xt_nfacct.c
@@ -6,6 +6,8 @@
  * it under the terms of the GNU General Public License version 2 (or any
  * later at your option) as published by the Free Software Foundation.
  */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/skbuff.h>
 
@@ -39,8 +41,8 @@ nfacct_mt_checkentry(const struct xt_mtchk_param *par)
 
 	nfacct = nfnl_acct_find_get(par->net, info->name);
 	if (nfacct == NULL) {
-		pr_info("xt_nfacct: accounting object with name `%s' "
-			"does not exists\n", info->name);
+		pr_info_ratelimited("accounting object `%s' does not exists\n",
+				    info->name);
 		return -ENOENT;
 	}
 	info->nfacct = nfacct;
diff --git a/net/netfilter/xt_physdev.c b/net/netfilter/xt_physdev.c
index bb33598e4530..9d6d67b953ac 100644
--- a/net/netfilter/xt_physdev.c
+++ b/net/netfilter/xt_physdev.c
@@ -107,9 +107,7 @@ static int physdev_mt_check(const struct xt_mtchk_param *par)
 	     info->invert & XT_PHYSDEV_OP_BRIDGED) &&
 	    par->hook_mask & ((1 << NF_INET_LOCAL_OUT) |
 	    (1 << NF_INET_FORWARD) | (1 << NF_INET_POST_ROUTING))) {
-		pr_info("using --physdev-out and --physdev-is-out are only "
-			"supported in the FORWARD and POSTROUTING chains with "
-			"bridged traffic.\n");
+		pr_info_ratelimited("--physdev-out and --physdev-is-out only supported in the FORWARD and POSTROUTING chains with bridged traffic\n");
 		if (par->hook_mask & (1 << NF_INET_LOCAL_OUT))
 			return -EINVAL;
 	}
diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
index db6a2d43bb30..453025fcf661 100644
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -357,8 +357,8 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
 	if ((info->check_set & XT_RECENT_REAP) && !info->seconds)
 		return -EINVAL;
 	if (info->hit_count >= XT_RECENT_MAX_NSTAMPS) {
-		pr_info("hitcount (%u) is larger than allowed maximum (%u)\n",
-			info->hit_count, XT_RECENT_MAX_NSTAMPS - 1);
+		pr_info_ratelimited("hitcount (%u) is larger than allowed maximum (%u)\n",
+				    info->hit_count, XT_RECENT_MAX_NSTAMPS - 1);
 		return -EINVAL;
 	}
 	if (info->name[0] == '\0' ||
@@ -587,7 +587,7 @@ recent_mt_proc_write(struct file *file, const char __user *input,
 		add = true;
 		break;
 	default:
-		pr_info("Need \"+ip\", \"-ip\" or \"/\"\n");
+		pr_info_ratelimited("Need \"+ip\", \"-ip\" or \"/\"\n");
 		return -EINVAL;
 	}
 
@@ -601,10 +601,8 @@ recent_mt_proc_write(struct file *file, const char __user *input,
 		succ   = in4_pton(c, size, (void *)&addr, '\n', NULL);
 	}
 
-	if (!succ) {
-		pr_info("illegal address written to procfs\n");
+	if (!succ)
 		return -EINVAL;
-	}
 
 	spin_lock_bh(&recent_lock);
 	e = recent_entry_lookup(t, &addr, family, 0);
diff --git a/net/netfilter/xt_set.c b/net/netfilter/xt_set.c
index 16b6b11ee83f..ba94286f25aa 100644
--- a/net/netfilter/xt_set.c
+++ b/net/netfilter/xt_set.c
@@ -92,12 +92,12 @@ set_match_v0_checkentry(const struct xt_mtchk_param *par)
 	index = ip_set_nfnl_get_byindex(par->net, info->match_set.index);
 
 	if (index == IPSET_INVALID_ID) {
-		pr_warn("Cannot find set identified by id %u to match\n",
-			info->match_set.index);
+		pr_warn_ratelimited("Cannot find set identified by id %u to match\n",
+				    info->match_set.index);
 		return -ENOENT;
 	}
 	if (info->match_set.u.flags[IPSET_DIM_MAX - 1] != 0) {
-		pr_warn("Protocol error: set match dimension is over the limit!\n");
+		pr_warn_ratelimited("set match dimension is over the limit!\n");
 		ip_set_nfnl_put(par->net, info->match_set.index);
 		return -ERANGE;
 	}
@@ -143,12 +143,12 @@ set_match_v1_checkentry(const struct xt_mtchk_param *par)
 	index = ip_set_nfnl_get_byindex(par->net, info->match_set.index);
 
 	if (index == IPSET_INVALID_ID) {
-		pr_warn("Cannot find set identified by id %u to match\n",
-			info->match_set.index);
+		pr_warn_ratelimited("Cannot find set identified by id %u to match\n",
+				    info->match_set.index);
 		return -ENOENT;
 	}
 	if (info->match_set.dim > IPSET_DIM_MAX) {
-		pr_warn("Protocol error: set match dimension is over the limit!\n");
+		pr_warn_ratelimited("set match dimension is over the limit!\n");
 		ip_set_nfnl_put(par->net, info->match_set.index);
 		return -ERANGE;
 	}
@@ -241,8 +241,8 @@ set_target_v0_checkentry(const struct xt_tgchk_param *par)
 	if (info->add_set.index != IPSET_INVALID_ID) {
 		index = ip_set_nfnl_get_byindex(par->net, info->add_set.index);
 		if (index == IPSET_INVALID_ID) {
-			pr_warn("Cannot find add_set index %u as target\n",
-				info->add_set.index);
+			pr_warn_ratelimited("Cannot find add_set index %u as target\n",
+					    info->add_set.index);
 			return -ENOENT;
 		}
 	}
@@ -250,8 +250,8 @@ set_target_v0_checkentry(const struct xt_tgchk_param *par)
 	if (info->del_set.index != IPSET_INVALID_ID) {
 		index = ip_set_nfnl_get_byindex(par->net, info->del_set.index);
 		if (index == IPSET_INVALID_ID) {
-			pr_warn("Cannot find del_set index %u as target\n",
-				info->del_set.index);
+			pr_warn_ratelimited("Cannot find del_set index %u as target\n",
+					    info->del_set.index);
 			if (info->add_set.index != IPSET_INVALID_ID)
 				ip_set_nfnl_put(par->net, info->add_set.index);
 			return -ENOENT;
@@ -259,7 +259,7 @@ set_target_v0_checkentry(const struct xt_tgchk_param *par)
 	}
 	if (info->add_set.u.flags[IPSET_DIM_MAX - 1] != 0 ||
 	    info->del_set.u.flags[IPSET_DIM_MAX - 1] != 0) {
-		pr_warn("Protocol error: SET target dimension is over the limit!\n");
+		pr_warn_ratelimited("SET target dimension over the limit!\n");
 		if (info->add_set.index != IPSET_INVALID_ID)
 			ip_set_nfnl_put(par->net, info->add_set.index);
 		if (info->del_set.index != IPSET_INVALID_ID)
@@ -316,8 +316,8 @@ set_target_v1_checkentry(const struct xt_tgchk_param *par)
 	if (info->add_set.index != IPSET_INVALID_ID) {
 		index = ip_set_nfnl_get_byindex(par->net, info->add_set.index);
 		if (index == IPSET_INVALID_ID) {
-			pr_warn("Cannot find add_set index %u as target\n",
-				info->add_set.index);
+			pr_warn_ratelimited("Cannot find add_set index %u as target\n",
+					    info->add_set.index);
 			return -ENOENT;
 		}
 	}
@@ -325,8 +325,8 @@ set_target_v1_checkentry(const struct xt_tgchk_param *par)
 	if (info->del_set.index != IPSET_INVALID_ID) {
 		index = ip_set_nfnl_get_byindex(par->net, info->del_set.index);
 		if (index == IPSET_INVALID_ID) {
-			pr_warn("Cannot find del_set index %u as target\n",
-				info->del_set.index);
+			pr_warn_ratelimited("Cannot find del_set index %u as target\n",
+					    info->del_set.index);
 			if (info->add_set.index != IPSET_INVALID_ID)
 				ip_set_nfnl_put(par->net, info->add_set.index);
 			return -ENOENT;
@@ -334,7 +334,7 @@ set_target_v1_checkentry(const struct xt_tgchk_param *par)
 	}
 	if (info->add_set.dim > IPSET_DIM_MAX ||
 	    info->del_set.dim > IPSET_DIM_MAX) {
-		pr_warn("Protocol error: SET target dimension is over the limit!\n");
+		pr_warn_ratelimited("SET target dimension over the limit!\n");
 		if (info->add_set.index != IPSET_INVALID_ID)
 			ip_set_nfnl_put(par->net, info->add_set.index);
 		if (info->del_set.index != IPSET_INVALID_ID)
@@ -444,8 +444,8 @@ set_target_v3_checkentry(const struct xt_tgchk_param *par)
 		index = ip_set_nfnl_get_byindex(par->net,
 						info->add_set.index);
 		if (index == IPSET_INVALID_ID) {
-			pr_warn("Cannot find add_set index %u as target\n",
-				info->add_set.index);
+			pr_warn_ratelimited("Cannot find add_set index %u as target\n",
+					    info->add_set.index);
 			return -ENOENT;
 		}
 	}
@@ -454,8 +454,8 @@ set_target_v3_checkentry(const struct xt_tgchk_param *par)
 		index = ip_set_nfnl_get_byindex(par->net,
 						info->del_set.index);
 		if (index == IPSET_INVALID_ID) {
-			pr_warn("Cannot find del_set index %u as target\n",
-				info->del_set.index);
+			pr_warn_ratelimited("Cannot find del_set index %u as target\n",
+					    info->del_set.index);
 			if (info->add_set.index != IPSET_INVALID_ID)
 				ip_set_nfnl_put(par->net,
 						info->add_set.index);
@@ -465,7 +465,7 @@ set_target_v3_checkentry(const struct xt_tgchk_param *par)
 
 	if (info->map_set.index != IPSET_INVALID_ID) {
 		if (strncmp(par->table, "mangle", 7)) {
-			pr_warn("--map-set only usable from mangle table\n");
+			pr_warn_ratelimited("--map-set only usable from mangle table\n");
 			return -EINVAL;
 		}
 		if (((info->flags & IPSET_FLAG_MAP_SKBPRIO) |
@@ -473,14 +473,14 @@ set_target_v3_checkentry(const struct xt_tgchk_param *par)
 		     !(par->hook_mask & (1 << NF_INET_FORWARD |
 					 1 << NF_INET_LOCAL_OUT |
 					 1 << NF_INET_POST_ROUTING))) {
-			pr_warn("mapping of prio or/and queue is allowed only from OUTPUT/FORWARD/POSTROUTING chains\n");
+			pr_warn_ratelimited("mapping of prio or/and queue is allowed only from OUTPUT/FORWARD/POSTROUTING chains\n");
 			return -EINVAL;
 		}
 		index = ip_set_nfnl_get_byindex(par->net,
 						info->map_set.index);
 		if (index == IPSET_INVALID_ID) {
-			pr_warn("Cannot find map_set index %u as target\n",
-				info->map_set.index);
+			pr_warn_ratelimited("Cannot find map_set index %u as target\n",
+					    info->map_set.index);
 			if (info->add_set.index != IPSET_INVALID_ID)
 				ip_set_nfnl_put(par->net,
 						info->add_set.index);
@@ -494,7 +494,7 @@ set_target_v3_checkentry(const struct xt_tgchk_param *par)
 	if (info->add_set.dim > IPSET_DIM_MAX ||
 	    info->del_set.dim > IPSET_DIM_MAX ||
 	    info->map_set.dim > IPSET_DIM_MAX) {
-		pr_warn("Protocol error: SET target dimension is over the limit!\n");
+		pr_warn_ratelimited("SET target dimension over the limit!\n");
 		if (info->add_set.index != IPSET_INVALID_ID)
 			ip_set_nfnl_put(par->net, info->add_set.index);
 		if (info->del_set.index != IPSET_INVALID_ID)
diff --git a/net/netfilter/xt_state.c b/net/netfilter/xt_state.c
index 5fbd79194d21..0b41c0befe3c 100644
--- a/net/netfilter/xt_state.c
+++ b/net/netfilter/xt_state.c
@@ -44,8 +44,8 @@ static int state_mt_check(const struct xt_mtchk_param *par)
 
 	ret = nf_ct_netns_get(par->net, par->family);
 	if (ret < 0)
-		pr_info("cannot load conntrack support for proto=%u\n",
-			par->family);
+		pr_info_ratelimited("cannot load conntrack support for proto=%u\n",
+				    par->family);
 	return ret;
 }
 
diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
index aea2b5a12ed7..894709391f90 100644
--- a/net/netfilter/xt_time.c
+++ b/net/netfilter/xt_time.c
@@ -235,8 +235,7 @@ static int time_mt_check(const struct xt_mtchk_param *par)
 
 	if (info->daytime_start > XT_TIME_MAX_DAYTIME ||
 	    info->daytime_stop > XT_TIME_MAX_DAYTIME) {
-		pr_info("invalid argument - start or "
-			"stop time greater than 23:59:59\n");
+		pr_info_ratelimited("invalid argument - start or stop time greater than 23:59:59\n");
 		return -EDOM;
 	}
 
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH nf 7/7] netfilter: x_tables: use pr ratelimiting in all remaining spots
  2018-02-07 13:48 ` [PATCH nf 7/7] netfilter: x_tables: use pr ratelimiting in all remaining spots Florian Westphal
@ 2018-02-07 17:00   ` Pablo Neira Ayuso
  2018-02-07 19:23     ` Florian Westphal
  0 siblings, 1 reply; 16+ messages in thread
From: Pablo Neira Ayuso @ 2018-02-07 17:00 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

Hi Florian,

Thanks for looking into this, comments below.

On Wed, Feb 07, 2018 at 02:48:28PM +0100, Florian Westphal wrote:
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  net/bridge/netfilter/ebt_among.c | 10 ++++----
>  net/bridge/netfilter/ebt_limit.c |  4 ++--
>  net/ipv4/netfilter/ipt_ECN.c     |  2 +-
>  net/ipv4/netfilter/ipt_REJECT.c  |  4 ++--
>  net/ipv6/netfilter/ip6t_REJECT.c |  4 ++--
>  net/ipv6/netfilter/ip6t_srh.c    |  6 +++--
>  net/netfilter/xt_AUDIT.c         |  4 ++--
>  net/netfilter/xt_CHECKSUM.c      |  5 ++--
>  net/netfilter/xt_CONNSECMARK.c   |  6 ++---
>  net/netfilter/xt_DSCP.c          |  2 +-
>  net/netfilter/xt_LED.c           |  2 +-
>  net/netfilter/xt_NFQUEUE.c       |  6 ++---
>  net/netfilter/xt_SECMARK.c       | 12 ++++++----
>  net/netfilter/xt_TCPMSS.c        | 10 ++++----
>  net/netfilter/xt_TPROXY.c        |  6 ++---
>  net/netfilter/xt_cgroup.c        |  8 ++++---
>  net/netfilter/xt_cluster.c       |  8 +++----
>  net/netfilter/xt_connbytes.c     |  4 ++--
>  net/netfilter/xt_connlabel.c     |  4 ++--
>  net/netfilter/xt_connmark.c      |  8 +++----
>  net/netfilter/xt_conntrack.c     |  4 ++--
>  net/netfilter/xt_dscp.c          |  2 +-
>  net/netfilter/xt_ecn.c           |  4 ++--
>  net/netfilter/xt_hashlimit.c     | 24 ++++++++++---------
>  net/netfilter/xt_helper.c        |  4 ++--
>  net/netfilter/xt_l2tp.c          | 20 +++++++++-------
>  net/netfilter/xt_limit.c         |  4 ++--
>  net/netfilter/xt_nat.c           |  5 ++--
>  net/netfilter/xt_nfacct.c        |  6 +++--
>  net/netfilter/xt_physdev.c       |  4 +---
>  net/netfilter/xt_recent.c        | 10 ++++----
>  net/netfilter/xt_set.c           | 50 ++++++++++++++++++++--------------------
>  net/netfilter/xt_state.c         |  4 ++--
>  net/netfilter/xt_time.c          |  3 +--
>  34 files changed, 132 insertions(+), 127 deletions(-)
> 
> diff --git a/net/bridge/netfilter/ebt_among.c b/net/bridge/netfilter/ebt_among.c
> index 279527f8b1fe..12d850a3ea68 100644
> --- a/net/bridge/netfilter/ebt_among.c
> +++ b/net/bridge/netfilter/ebt_among.c
> @@ -187,17 +187,17 @@ static int ebt_among_mt_check(const struct xt_mtchk_param *par)
>  	expected_length += ebt_mac_wormhash_size(wh_src);
>  
>  	if (em->match_size != EBT_ALIGN(expected_length)) {
> -		pr_info("wrong size: %d against expected %d, rounded to %zd\n",
> -			em->match_size, expected_length,
> -			EBT_ALIGN(expected_length));
> +		pr_info_ratelimited("wrong size: %d against expected %d, rounded to %zd\n",

Shouldn't all these be pr_err_ratelimited instead?

Probably this is a good chance to homogeneize all error reporting in
xtables.

> +				    em->match_size, expected_length,
> +				    EBT_ALIGN(expected_length));
>  		return -EINVAL;
>  	}
>  	if (wh_dst && (err = ebt_mac_wormhash_check_integrity(wh_dst))) {
> -		pr_info("dst integrity fail: %x\n", -err);
> +		pr_info_ratelimited("dst integrity fail: %x\n", -err);
>  		return -EINVAL;
>  	}
>  	if (wh_src && (err = ebt_mac_wormhash_check_integrity(wh_src))) {
> -		pr_info("src integrity fail: %x\n", -err);
> +		pr_info_ratelimited("src integrity fail: %x\n", -err);
>  		return -EINVAL;
>  	}
>  	return 0;
[...]
> diff --git a/net/netfilter/xt_NFQUEUE.c b/net/netfilter/xt_NFQUEUE.c
> index a360b99a958a..9fac4710f7cf 100644
> --- a/net/netfilter/xt_NFQUEUE.c
> +++ b/net/netfilter/xt_NFQUEUE.c
> @@ -67,13 +67,13 @@ static int nfqueue_tg_check(const struct xt_tgchk_param *par)
>  	init_hashrandom(&jhash_initval);
>  
>  	if (info->queues_total == 0) {
> -		pr_err("NFQUEUE: number of total queues is 0\n");
                        ^^^^^^^^

We can probably add this all over the place in the same go?

        #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

> +		pr_err_ratelimited("NFQUEUE: number of total queues is 0\n");
>  		return -EINVAL;
>  	}
>  	maxid = info->queues_total - 1 + info->queuenum;
>  	if (maxid > 0xffff) {
> -		pr_err("NFQUEUE: number of queues (%u) out of range (got %u)\n",
> -		       info->queues_total, maxid);
> +		pr_err_ratelimited("NFQUEUE: number of queues (%u) out of range (got %u)\n",
> +				   info->queues_total, maxid);
>  		return -ERANGE;
>  	}
>  	if (par->target->revision == 2 && info->flags > 1)
[...]
> diff --git a/net/netfilter/xt_set.c b/net/netfilter/xt_set.c
> index 16b6b11ee83f..ba94286f25aa 100644
> --- a/net/netfilter/xt_set.c
> +++ b/net/netfilter/xt_set.c
> @@ -92,12 +92,12 @@ set_match_v0_checkentry(const struct xt_mtchk_param *par)
>  	index = ip_set_nfnl_get_byindex(par->net, info->match_set.index);
>  
>  	if (index == IPSET_INVALID_ID) {
> -		pr_warn("Cannot find set identified by id %u to match\n",
> -			info->match_set.index);
> +		pr_warn_ratelimited("Cannot find set identified by id %u to match\n",
> +				    info->match_set.index);

Use pr_err_ratelimited instead?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH nf 2/7] netfilter: x_tables: prefer pr_debug where possible
  2018-02-07 13:48 ` [PATCH nf 2/7] netfilter: x_tables: prefer pr_debug " Florian Westphal
@ 2018-02-07 17:02   ` Pablo Neira Ayuso
  2018-02-07 19:15     ` Florian Westphal
  0 siblings, 1 reply; 16+ messages in thread
From: Pablo Neira Ayuso @ 2018-02-07 17:02 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Wed, Feb 07, 2018 at 02:48:23PM +0100, Florian Westphal wrote:
> prefer pr_debug for cases where error is usually not seen by users.
> checkpatch complains due to lines > 80 but adding a newline doesn't
> make things any more readable.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  net/ipv4/netfilter/ipt_rpfilter.c  | 2 +-
>  net/ipv6/netfilter/ip6t_rpfilter.c | 2 +-
>  net/netfilter/xt_SECMARK.c         | 2 +-
>  net/netfilter/xt_bpf.c             | 2 +-
>  net/netfilter/xt_connlabel.c       | 2 +-
>  net/netfilter/xt_ipcomp.c          | 2 +-
>  net/netfilter/xt_ipvs.c            | 2 +-
>  net/netfilter/xt_l2tp.c            | 2 +-
>  net/netfilter/xt_recent.c          | 4 ++--
>  net/netfilter/xt_socket.c          | 8 ++++----
>  net/netfilter/xt_time.c            | 2 +-
>  11 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
> index 37fb9552e858..ffd1cf65af3a 100644
> --- a/net/ipv4/netfilter/ipt_rpfilter.c
> +++ b/net/ipv4/netfilter/ipt_rpfilter.c
> @@ -105,7 +105,7 @@ static int rpfilter_check(const struct xt_mtchk_param *par)
>  	const struct xt_rpfilter_info *info = par->matchinfo;
>  	unsigned int options = ~XT_RPFILTER_OPTION_MASK;
>  	if (info->flags & options) {
> -		pr_info("unknown options encountered");
> +		pr_debug("unknown options");

OK, so the idea is to use pr_debug() when it is unlikely to hit an
error via iptables, right?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH nf 1/7] netfilter: x_tables: remove pr_info where possible
  2018-02-07 13:48 ` [PATCH nf 1/7] netfilter: x_tables: remove pr_info where possible Florian Westphal
@ 2018-02-07 17:03   ` Pablo Neira Ayuso
  2018-02-07 19:14     ` Florian Westphal
  0 siblings, 1 reply; 16+ messages in thread
From: Pablo Neira Ayuso @ 2018-02-07 17:03 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Wed, Feb 07, 2018 at 02:48:22PM +0100, Florian Westphal wrote:
> remove several pr_info messages that cannot be triggered with iptables.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  net/ipv4/netfilter/ipt_ECN.c | 10 ++++------
>  net/netfilter/xt_HL.c        | 13 +++----------
>  net/netfilter/xt_LED.c       |  4 +---
>  net/netfilter/xt_cgroup.c    |  4 +---
>  4 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/ipt_ECN.c b/net/ipv4/netfilter/ipt_ECN.c
> index 270765236f5e..39ff167e6d86 100644
> --- a/net/ipv4/netfilter/ipt_ECN.c
> +++ b/net/ipv4/netfilter/ipt_ECN.c
> @@ -98,14 +98,12 @@ static int ecn_tg_check(const struct xt_tgchk_param *par)
>  	const struct ipt_ECN_info *einfo = par->targinfo;
>  	const struct ipt_entry *e = par->entryinfo;
>  
> -	if (einfo->operation & IPT_ECN_OP_MASK) {
> -		pr_info("unsupported ECN operation %x\n", einfo->operation);
> +	if (einfo->operation & IPT_ECN_OP_MASK)

According to patch 2/7, these should be pr_debug(), or probably I'm
misunderstanding something :-).

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH nf 1/7] netfilter: x_tables: remove pr_info where possible
  2018-02-07 17:03   ` Pablo Neira Ayuso
@ 2018-02-07 19:14     ` Florian Westphal
  0 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 19:14 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Feb 07, 2018 at 02:48:22PM +0100, Florian Westphal wrote:
> > remove several pr_info messages that cannot be triggered with iptables.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> >  net/ipv4/netfilter/ipt_ECN.c | 10 ++++------
> >  net/netfilter/xt_HL.c        | 13 +++----------
> >  net/netfilter/xt_LED.c       |  4 +---
> >  net/netfilter/xt_cgroup.c    |  4 +---
> >  4 files changed, 9 insertions(+), 22 deletions(-)
> > 
> > diff --git a/net/ipv4/netfilter/ipt_ECN.c b/net/ipv4/netfilter/ipt_ECN.c
> > index 270765236f5e..39ff167e6d86 100644
> > --- a/net/ipv4/netfilter/ipt_ECN.c
> > +++ b/net/ipv4/netfilter/ipt_ECN.c
> > @@ -98,14 +98,12 @@ static int ecn_tg_check(const struct xt_tgchk_param *par)
> >  	const struct ipt_ECN_info *einfo = par->targinfo;
> >  	const struct ipt_entry *e = par->entryinfo;
> >  
> > -	if (einfo->operation & IPT_ECN_OP_MASK) {
> > -		pr_info("unsupported ECN operation %x\n", einfo->operation);
> > +	if (einfo->operation & IPT_ECN_OP_MASK)
> 
> According to patch 2/7, these should be pr_debug(), or probably I'm
> misunderstanding something :-).

Right, there is no consistency in the tree currently.

I don't think we'll see any new options added to ipt_ECN, so I don't
think its worth having a pr_foo() for this.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH nf 2/7] netfilter: x_tables: prefer pr_debug where possible
  2018-02-07 17:02   ` Pablo Neira Ayuso
@ 2018-02-07 19:15     ` Florian Westphal
  0 siblings, 0 replies; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 19:15 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Feb 07, 2018 at 02:48:23PM +0100, Florian Westphal wrote:
> > prefer pr_debug for cases where error is usually not seen by users.
> > checkpatch complains due to lines > 80 but adding a newline doesn't
> > make things any more readable.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> >  net/ipv4/netfilter/ipt_rpfilter.c  | 2 +-
> >  net/ipv6/netfilter/ip6t_rpfilter.c | 2 +-
> >  net/netfilter/xt_SECMARK.c         | 2 +-
> >  net/netfilter/xt_bpf.c             | 2 +-
> >  net/netfilter/xt_connlabel.c       | 2 +-
> >  net/netfilter/xt_ipcomp.c          | 2 +-
> >  net/netfilter/xt_ipvs.c            | 2 +-
> >  net/netfilter/xt_l2tp.c            | 2 +-
> >  net/netfilter/xt_recent.c          | 4 ++--
> >  net/netfilter/xt_socket.c          | 8 ++++----
> >  net/netfilter/xt_time.c            | 2 +-
> >  11 files changed, 15 insertions(+), 15 deletions(-)
> > 
> > diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
> > index 37fb9552e858..ffd1cf65af3a 100644
> > --- a/net/ipv4/netfilter/ipt_rpfilter.c
> > +++ b/net/ipv4/netfilter/ipt_rpfilter.c
> > @@ -105,7 +105,7 @@ static int rpfilter_check(const struct xt_mtchk_param *par)
> >  	const struct xt_rpfilter_info *info = par->matchinfo;
> >  	unsigned int options = ~XT_RPFILTER_OPTION_MASK;
> >  	if (info->flags & options) {
> > -		pr_info("unknown options encountered");
> > +		pr_debug("unknown options");
> 
> OK, so the idea is to use pr_debug() when it is unlikely to hit an
> error via iptables, right?

Yes, alternatively this pr_* could be removed.

Theoretically we could have some new version of iptables hat support
--rpfilter-foobar flag which would then trigger this -EINVAL.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH nf 7/7] netfilter: x_tables: use pr ratelimiting in all remaining spots
  2018-02-07 17:00   ` Pablo Neira Ayuso
@ 2018-02-07 19:23     ` Florian Westphal
  2018-02-07 19:30       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 16+ messages in thread
From: Florian Westphal @ 2018-02-07 19:23 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > --- a/net/bridge/netfilter/ebt_among.c
> > +++ b/net/bridge/netfilter/ebt_among.c
> > @@ -187,17 +187,17 @@ static int ebt_among_mt_check(const struct xt_mtchk_param *par)
> >  	expected_length += ebt_mac_wormhash_size(wh_src);
> >  
> >  	if (em->match_size != EBT_ALIGN(expected_length)) {
> > -		pr_info("wrong size: %d against expected %d, rounded to %zd\n",
> > -			em->match_size, expected_length,
> > -			EBT_ALIGN(expected_length));
> > +		pr_info_ratelimited("wrong size: %d against expected %d, rounded to %zd\n",
> 
> Shouldn't all these be pr_err_ratelimited instead?

Don't know.

This could even be pr_debug actually since this message is
useless unless you're doing ebtables development work.

> Probably this is a good chance to homogeneize all error reporting in
> xtables.

Yes.

> >  	if (wh_dst && (err = ebt_mac_wormhash_check_integrity(wh_dst))) {
> > -		pr_info("dst integrity fail: %x\n", -err);
> > +		pr_info_ratelimited("dst integrity fail: %x\n", -err);
> >  		return -EINVAL;
> >  	}
> >  	if (wh_src && (err = ebt_mac_wormhash_check_integrity(wh_src))) {
> > -		pr_info("src integrity fail: %x\n", -err);
> > +		pr_info_ratelimited("src integrity fail: %x\n", -err);
> >  		return -EINVAL;

Same for these two, I'll convert all to pr_debug instead.

> >  	if (info->queues_total == 0) {
> > -		pr_err("NFQUEUE: number of total queues is 0\n");
>                         ^^^^^^^^
> 
> We can probably add this all over the place in the same go?
 
>         #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Yes.

> >  	if (index == IPSET_INVALID_ID) {
> > -		pr_warn("Cannot find set identified by id %u to match\n",
> > -			info->match_set.index);
> > +		pr_warn_ratelimited("Cannot find set identified by id %u to match\n",
> > +				    info->match_set.index);
> 
> Use pr_err_ratelimited instead?

I think we should settle on a single pr_foo, i suggest
pr_info(_ratelimited).

This is not an error condition, we only have these
printks because we can't return a proper error to userspace.

If this was netlink, it would be converted to extack instead...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH nf 7/7] netfilter: x_tables: use pr ratelimiting in all remaining spots
  2018-02-07 19:23     ` Florian Westphal
@ 2018-02-07 19:30       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2018-02-07 19:30 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Wed, Feb 07, 2018 at 08:23:23PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > --- a/net/bridge/netfilter/ebt_among.c
> > > +++ b/net/bridge/netfilter/ebt_among.c
> > > @@ -187,17 +187,17 @@ static int ebt_among_mt_check(const struct xt_mtchk_param *par)
> > >  	expected_length += ebt_mac_wormhash_size(wh_src);
> > >  
> > >  	if (em->match_size != EBT_ALIGN(expected_length)) {
> > > -		pr_info("wrong size: %d against expected %d, rounded to %zd\n",
> > > -			em->match_size, expected_length,
> > > -			EBT_ALIGN(expected_length));
> > > +		pr_info_ratelimited("wrong size: %d against expected %d, rounded to %zd\n",
> > 
> > Shouldn't all these be pr_err_ratelimited instead?
> 
> Don't know.
> 
> This could even be pr_debug actually since this message is
> useless unless you're doing ebtables development work.

I see, I'm telling this because iptables says 'look at dmesg' when we
hit EINVAL, but there will be nothing.

[...]
> > >  	if (index == IPSET_INVALID_ID) {
> > > -		pr_warn("Cannot find set identified by id %u to match\n",
> > > -			info->match_set.index);
> > > +		pr_warn_ratelimited("Cannot find set identified by id %u to match\n",
> > > +				    info->match_set.index);
> > 
> > Use pr_err_ratelimited instead?
> 
> I think we should settle on a single pr_foo, i suggest
> pr_info(_ratelimited).

OK.

> This is not an error condition, we only have these
> printks because we can't return a proper error to userspace.
> 
> If this was netlink, it would be converted to extack instead...

Indeed, we have this primitive error reporting in iptables, we can do
better in nftables.

Thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: netfilter: x_tables: ratelimit most printks
  2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
                   ` (6 preceding siblings ...)
  2018-02-07 13:48 ` [PATCH nf 7/7] netfilter: x_tables: use pr ratelimiting in all remaining spots Florian Westphal
@ 2018-02-14 19:49 ` Pablo Neira Ayuso
  7 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2018-02-14 19:49 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Wed, Feb 07, 2018 at 02:48:21PM +0100, Florian Westphal wrote:
> Aeons ago, before namespaces, there was no need to ratelimit this:
> all of these error messages got triggered in response to iptables
> commands, which need CAP_NET_ADMIN.
> 
> Nowadays we have namespaces, so its better to ratelimit these.
> This should also help fuzzing (syzkaller), as it can generate a large
> volume of error messages (which are useless there).
> 
> The patches are split as follows:
> - first get rid of printks that should never be triggered, as userland
>   doesn't generate such malformed rules anyway.
> - second, switch some printks to pr_debug.  This is mostly for messages
>   where it might make sense for developers to see what exactly went
>   wrong.
> 
> Rest of the patches swap remaining pr_foo with pr_foo_ratelimited().
> 
> Note that most patches introduce overly long lines, but splitting these
> would make it necessary to split the error messages which is worse.
> 
> 46 files changed, 254 insertions(+), 257 deletions(-)

Series applied, thanks Florian.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-02-14 19:49 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07 13:48 netfilter: x_tables: ratelimit most printks Florian Westphal
2018-02-07 13:48 ` [PATCH nf 1/7] netfilter: x_tables: remove pr_info where possible Florian Westphal
2018-02-07 17:03   ` Pablo Neira Ayuso
2018-02-07 19:14     ` Florian Westphal
2018-02-07 13:48 ` [PATCH nf 2/7] netfilter: x_tables: prefer pr_debug " Florian Westphal
2018-02-07 17:02   ` Pablo Neira Ayuso
2018-02-07 19:15     ` Florian Westphal
2018-02-07 13:48 ` [PATCH nf 3/7] netfilter: xt_CT: use pr ratelimiting Florian Westphal
2018-02-07 13:48 ` [PATCH nf 4/7] netfilter: x_tables: rate limit pr_err warnings Florian Westphal
2018-02-07 13:48 ` [PATCH nf 5/7] netfilter: x_tables: rate-limit table mismatch warnings Florian Westphal
2018-02-07 13:48 ` [PATCH nf 6/7] netfilter: x_tables: use pr ratelimiting Florian Westphal
2018-02-07 13:48 ` [PATCH nf 7/7] netfilter: x_tables: use pr ratelimiting in all remaining spots Florian Westphal
2018-02-07 17:00   ` Pablo Neira Ayuso
2018-02-07 19:23     ` Florian Westphal
2018-02-07 19:30       ` Pablo Neira Ayuso
2018-02-14 19:49 ` netfilter: x_tables: ratelimit most printks Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.