netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).