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

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.

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>