[nf PATCH] ipset: core: Hold module reference while requesting a module

[nf PATCH] ipset: core: Hold module reference while requesting a module

[Phil Sutter]

User space may unload ip_set.ko while it is itself requesting a set type
backend module, leading to a kernel crash. The race condition may be
provoked by inserting an mdelay() right after the nfnl_unlock() call.

Fixes: a7b4f989a629 ("netfilter: ipset: IP set core support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/netfilter/ipset/ip_set_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 61431690cbd5..cc20e6d56807 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -104,14 +104,19 @@ find_set_type(const char *name, u8 family, u8 revision)
 static bool
 load_settype(const char *name)
 {
+	if (!try_module_get(THIS_MODULE))
+		return false;
+
 	nfnl_unlock(NFNL_SUBSYS_IPSET);
 	pr_debug("try to load ip_set_%s\n", name);
 	if (request_module("ip_set_%s", name) < 0) {
 		pr_warn("Can't find ip_set type %s\n", name);
 		nfnl_lock(NFNL_SUBSYS_IPSET);
+		module_put(THIS_MODULE);
 		return false;
 	}
 	nfnl_lock(NFNL_SUBSYS_IPSET);
+	module_put(THIS_MODULE);
 	return true;
 }
 
-- 
2.47.0

Re: [nf PATCH] ipset: core: Hold module reference while requesting a module

[Jozsef Kadlecsik]

Hi Phil,

On Fri, 29 Nov 2024, Phil Sutter wrote:

> User space may unload ip_set.ko while it is itself requesting a set type
> backend module, leading to a kernel crash. The race condition may be
> provoked by inserting an mdelay() right after the nfnl_unlock() call.
> 
> Fixes: a7b4f989a629 ("netfilter: ipset: IP set core support")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Good catch! It's an unlocked area, so the module reference is indeed 
required.

Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>

Best regards,
Jozsef
> ---
>  net/netfilter/ipset/ip_set_core.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> index 61431690cbd5..cc20e6d56807 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -104,14 +104,19 @@ find_set_type(const char *name, u8 family, u8 revision)
>  static bool
>  load_settype(const char *name)
>  {
> +	if (!try_module_get(THIS_MODULE))
> +		return false;
> +
>  	nfnl_unlock(NFNL_SUBSYS_IPSET);
>  	pr_debug("try to load ip_set_%s\n", name);
>  	if (request_module("ip_set_%s", name) < 0) {
>  		pr_warn("Can't find ip_set type %s\n", name);
>  		nfnl_lock(NFNL_SUBSYS_IPSET);
> +		module_put(THIS_MODULE);
>  		return false;
>  	}
>  	nfnl_lock(NFNL_SUBSYS_IPSET);
> +	module_put(THIS_MODULE);
>  	return true;
>  }
>  
> -- 
> 2.47.0
> 
> 

-- 
E-mail : kadlec@netfilter.org, kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.hu
Address: Wigner Research Centre for Physics
         H-1525 Budapest 114, POB. 49, Hungary

Re: [nf PATCH] ipset: core: Hold module reference while requesting a module

[Pablo Neira Ayuso]

On Fri, Nov 29, 2024 at 04:30:38PM +0100, Phil Sutter wrote:
> User space may unload ip_set.ko while it is itself requesting a set type
> backend module, leading to a kernel crash. The race condition may be
> provoked by inserting an mdelay() right after the nfnl_unlock()
> call.

Applied, thanks