* [PATCH] netfilter: xt_hashlimit: fix inconsistent return type in hashlimit_mt_*
@ 2025-08-29 12:51 Miaoqian Lin
2025-08-29 13:18 ` Phil Sutter
2025-08-29 14:16 ` Florian Westphal
0 siblings, 2 replies; 3+ messages in thread
From: Miaoqian Lin @ 2025-08-29 12:51 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Joshua Hunt, Vishwanath Pai, netfilter-devel, coreteam, netdev,
linux-kernel
Cc: linmq006
The hashlimit_mt_v1() and hashlimit_mt_v2() functions return the
cfg_copy() error code (-EINVAL) instead of false when configuration
copying fails. Since these functions are declared to return bool,
-EINVAL is interpreted as true, which is misleading.
Fixes: 11d5f15723c9 ("netfilter: xt_hashlimit: Create revision 2 to support higher pps rates")
Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
net/netfilter/xt_hashlimit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 3b507694e81e..de54d8f37852 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -806,7 +806,7 @@ hashlimit_mt_v1(const struct sk_buff *skb, struct xt_action_param *par)
ret = cfg_copy(&cfg, (void *)&info->cfg, 1);
if (ret)
- return ret;
+ return false;
return hashlimit_mt_common(skb, par, hinfo, &cfg, 1);
}
@@ -821,7 +821,7 @@ hashlimit_mt_v2(const struct sk_buff *skb, struct xt_action_param *par)
ret = cfg_copy(&cfg, (void *)&info->cfg, 2);
if (ret)
- return ret;
+ return false;
return hashlimit_mt_common(skb, par, hinfo, &cfg, 2);
}
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] netfilter: xt_hashlimit: fix inconsistent return type in hashlimit_mt_*
2025-08-29 12:51 [PATCH] netfilter: xt_hashlimit: fix inconsistent return type in hashlimit_mt_* Miaoqian Lin
@ 2025-08-29 13:18 ` Phil Sutter
2025-08-29 14:16 ` Florian Westphal
1 sibling, 0 replies; 3+ messages in thread
From: Phil Sutter @ 2025-08-29 13:18 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Joshua Hunt, Vishwanath Pai, netfilter-devel, coreteam, netdev,
linux-kernel
On Fri, Aug 29, 2025 at 08:51:31PM +0800, Miaoqian Lin wrote:
> The hashlimit_mt_v1() and hashlimit_mt_v2() functions return the
> cfg_copy() error code (-EINVAL) instead of false when configuration
> copying fails. Since these functions are declared to return bool,
> -EINVAL is interpreted as true, which is misleading.
>
> Fixes: 11d5f15723c9 ("netfilter: xt_hashlimit: Create revision 2 to support higher pps rates")
> Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
Reviewed-by: Phil Sutter <phil@nwl.cc>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] netfilter: xt_hashlimit: fix inconsistent return type in hashlimit_mt_*
2025-08-29 12:51 [PATCH] netfilter: xt_hashlimit: fix inconsistent return type in hashlimit_mt_* Miaoqian Lin
2025-08-29 13:18 ` Phil Sutter
@ 2025-08-29 14:16 ` Florian Westphal
1 sibling, 0 replies; 3+ messages in thread
From: Florian Westphal @ 2025-08-29 14:16 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Joshua Hunt, Vishwanath Pai, netfilter-devel, coreteam, netdev,
linux-kernel
Miaoqian Lin <linmq006@gmail.com> wrote:
> The hashlimit_mt_v1() and hashlimit_mt_v2() functions return the
> cfg_copy() error code (-EINVAL) instead of false when configuration
> copying fails. Since these functions are declared to return bool,
> -EINVAL is interpreted as true, which is misleading.
Could you please check if its possible to rework cfg_copy() to not
return anything?
> --- a/net/netfilter/xt_hashlimit.c
> +++ b/net/netfilter/xt_hashlimit.c
> @@ -806,7 +806,7 @@ hashlimit_mt_v1(const struct sk_buff *skb, struct xt_action_param *par)
>
> ret = cfg_copy(&cfg, (void *)&info->cfg, 1);
> if (ret)
> - return ret;
> + return false;
AFAICS cfg_copy cannot return an error.
You could try adding an enum for the version field to xt_hashlimit.c,
then use switch/case to let compiler complain for other values.
Or try to replace the else branch error return with BUILD_BUG(),
compiler should be able to figure this out.
You might have to add __always_inline hint.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-29 14:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 12:51 [PATCH] netfilter: xt_hashlimit: fix inconsistent return type in hashlimit_mt_* Miaoqian Lin
2025-08-29 13:18 ` Phil Sutter
2025-08-29 14:16 ` Florian Westphal
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).