netfilter-devel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).