* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
@ 2024-10-23 17:07 kernel test robot
0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2024-10-23 17:07 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20241022085753.2069639-1-dongchenchen2@huawei.com>
References: <20241022085753.2069639-1-dongchenchen2@huawei.com>
TO: Dong Chenchen <dongchenchen2@huawei.com>
TO: pablo@netfilter.org
TO: kadlec@netfilter.org
TO: davem@davemloft.net
TO: edumazet@google.com
TO: kuba@kernel.org
TO: pabeni@redhat.com
TO: fw@strlen.de
TO: kuniyu@amazon.com
CC: netfilter-devel@vger.kernel.org
CC: netdev@vger.kernel.org
CC: yuehaibing@huawei.com
CC: Dong Chenchen <dongchenchen2@huawei.com>
Hi Dong,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/Dong-Chenchen/net-netfilter-Fix-use-after-free-in-get_info/20241022-165936
base: net/main
patch link: https://lore.kernel.org/r/20241022085753.2069639-1-dongchenchen2%40huawei.com
patch subject: [PATCH net] net: netfilter: Fix use-after-free in get_info()
:::::: branch date: 32 hours ago
:::::: commit date: 32 hours ago
config: x86_64-randconfig-161-20241023 (https://download.01.org/0day-ci/archive/20241024/202410240020.Cqi2d68p-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202410240020.Cqi2d68p-lkp@intel.com/
smatch warnings:
net/netfilter/x_tables.c:1280 xt_find_table_lock() warn: passing zero to 'ERR_PTR'
vim +/ERR_PTR +1280 net/netfilter/x_tables.c
1ef4d6d1af2d0c Florian Westphal 2021-04-21 1233
03d13b6868a261 Florian Westphal 2017-12-08 1234 /* Find table by name, grabs mutex & ref. Returns ERR_PTR on error. */
76108cea065cda Jan Engelhardt 2008-10-08 1235 struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
76108cea065cda Jan Engelhardt 2008-10-08 1236 const char *name)
2e4e6a17af35be Harald Welte 2006-01-12 1237 {
1d610d4d31a8ed Florian Westphal 2021-04-01 1238 struct xt_pernet *xt_net = net_generic(net, xt_pernet_id);
fdacd57c79b79a Florian Westphal 2021-08-03 1239 struct module *owner = NULL;
fdacd57c79b79a Florian Westphal 2021-08-03 1240 struct xt_template *tmpl;
fdacd57c79b79a Florian Westphal 2021-08-03 1241 struct xt_table *t;
f4f502d5a8ea29 Dong Chenchen 2024-10-22 1242 int err = -ENOENT;
2e4e6a17af35be Harald Welte 2006-01-12 1243
7926dbfa4bc14e Pablo Neira Ayuso 2014-07-31 1244 mutex_lock(&xt[af].mutex);
1d610d4d31a8ed Florian Westphal 2021-04-01 1245 list_for_each_entry(t, &xt_net->tables[af], list)
2e4e6a17af35be Harald Welte 2006-01-12 1246 if (strcmp(t->name, name) == 0 && try_module_get(t->me))
2e4e6a17af35be Harald Welte 2006-01-12 1247 return t;
b9e69e12739718 Florian Westphal 2016-02-25 1248
fdacd57c79b79a Florian Westphal 2021-08-03 1249 /* Table doesn't exist in this netns, check larval list */
fdacd57c79b79a Florian Westphal 2021-08-03 1250 list_for_each_entry(tmpl, &xt_templates[af], list) {
fdacd57c79b79a Florian Westphal 2021-08-03 1251 if (strcmp(tmpl->name, name))
b9e69e12739718 Florian Westphal 2016-02-25 1252 continue;
fdacd57c79b79a Florian Westphal 2021-08-03 1253 if (!try_module_get(tmpl->me))
03d13b6868a261 Florian Westphal 2017-12-08 1254 goto out;
fdacd57c79b79a Florian Westphal 2021-08-03 1255
fdacd57c79b79a Florian Westphal 2021-08-03 1256 owner = tmpl->me;
fdacd57c79b79a Florian Westphal 2021-08-03 1257
b9e69e12739718 Florian Westphal 2016-02-25 1258 mutex_unlock(&xt[af].mutex);
fdacd57c79b79a Florian Westphal 2021-08-03 1259 err = tmpl->table_init(net);
03d13b6868a261 Florian Westphal 2017-12-08 1260 if (err < 0) {
fdacd57c79b79a Florian Westphal 2021-08-03 1261 module_put(owner);
03d13b6868a261 Florian Westphal 2017-12-08 1262 return ERR_PTR(err);
b9e69e12739718 Florian Westphal 2016-02-25 1263 }
b9e69e12739718 Florian Westphal 2016-02-25 1264
b9e69e12739718 Florian Westphal 2016-02-25 1265 mutex_lock(&xt[af].mutex);
b9e69e12739718 Florian Westphal 2016-02-25 1266 break;
b9e69e12739718 Florian Westphal 2016-02-25 1267 }
b9e69e12739718 Florian Westphal 2016-02-25 1268
f4f502d5a8ea29 Dong Chenchen 2024-10-22 1269 if (err < 0)
f4f502d5a8ea29 Dong Chenchen 2024-10-22 1270 goto out;
f4f502d5a8ea29 Dong Chenchen 2024-10-22 1271
b9e69e12739718 Florian Westphal 2016-02-25 1272 /* and once again: */
1d610d4d31a8ed Florian Westphal 2021-04-01 1273 list_for_each_entry(t, &xt_net->tables[af], list)
b9e69e12739718 Florian Westphal 2016-02-25 1274 if (strcmp(t->name, name) == 0)
b9e69e12739718 Florian Westphal 2016-02-25 1275 return t;
b9e69e12739718 Florian Westphal 2016-02-25 1276
fdacd57c79b79a Florian Westphal 2021-08-03 1277 module_put(owner);
b9e69e12739718 Florian Westphal 2016-02-25 1278 out:
9e19bb6d7a0959 Ingo Molnar 2006-03-25 1279 mutex_unlock(&xt[af].mutex);
f4f502d5a8ea29 Dong Chenchen 2024-10-22 @1280 return ERR_PTR(err);
2e4e6a17af35be Harald Welte 2006-01-12 1281 }
2e4e6a17af35be Harald Welte 2006-01-12 1282 EXPORT_SYMBOL_GPL(xt_find_table_lock);
2e4e6a17af35be Harald Welte 2006-01-12 1283
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
@ 2024-10-23 1:28 dongchenchen (A)
0 siblings, 0 replies; 11+ messages in thread
From: dongchenchen (A) @ 2024-10-23 1:28 UTC (permalink / raw)
To: Florian Westphal
Cc: pablo@netfilter.org, kadlec@netfilter.org, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
kuniyu@amazon.com, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org, yuehaibing
> Dong Chenchen <dongchenchen2@huawei.com> wrote:
> > net/netfilter/x_tables.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index
> > da5d929c7c85..359c880ecb07 100644
> > --- a/net/netfilter/x_tables.c
> > +++ b/net/netfilter/x_tables.c
> > @@ -1239,6 +1239,7 @@ struct xt_table *xt_find_table_lock(struct net *net,
> u_int8_t af,
> > struct module *owner = NULL;
> > struct xt_template *tmpl;
> > struct xt_table *t;
> > + int err = -ENOENT;
> >
> > mutex_lock(&xt[af].mutex);
> > list_for_each_entry(t, &xt_net->tables[af], list) @@ -1247,8 +1248,6
> > @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> >
> > /* Table doesn't exist in this netns, check larval list */
> > list_for_each_entry(tmpl, &xt_templates[af], list) {
> > - int err;
> > -
> > if (strcmp(tmpl->name, name))
> > continue;
> > if (!try_module_get(tmpl->me))
> > @@ -1267,6 +1266,9 @@ struct xt_table *xt_find_table_lock(struct net *net,
> u_int8_t af,
> > break;
> > }
> >
> > + if (err < 0)
> > + goto out;
> > +
> > /* and once again: */
> > list_for_each_entry(t, &xt_net->tables[af], list)
> > if (strcmp(t->name, name) == 0)
>
> Proabably also:
>
> - if (strcmp(t->name, name) == 0)
> + if (strcmp(t->name, name) == 0 && owner == t->me)
>
Thank you very much for your suggestions!
V2 will be sent.
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH net] net: netfilter: Fix use-after-free in get_info()
@ 2024-10-22 8:57 Dong Chenchen
2024-10-22 9:22 ` Yue Haibing
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Dong Chenchen @ 2024-10-22 8:57 UTC (permalink / raw)
To: pablo, kadlec, davem, edumazet, kuba, pabeni, fw, kuniyu
Cc: netfilter-devel, netdev, yuehaibing, Dong Chenchen
ip6table_nat module unload has refcnt warning for UAF. call trace is:
WARNING: CPU: 1 PID: 379 at kernel/module/main.c:853 module_put+0x6f/0x80
Modules linked in: ip6table_nat(-)
CPU: 1 UID: 0 PID: 379 Comm: ip6tables Not tainted 6.12.0-rc4-00047-gc2ee9f594da8-dirty #205
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
RIP: 0010:module_put+0x6f/0x80
Call Trace:
<TASK>
get_info+0x128/0x180
do_ip6t_get_ctl+0x6a/0x430
nf_getsockopt+0x46/0x80
ipv6_getsockopt+0xb9/0x100
rawv6_getsockopt+0x42/0x190
do_sock_getsockopt+0xaa/0x180
__sys_getsockopt+0x70/0xc0
__x64_sys_getsockopt+0x20/0x30
do_syscall_64+0xa2/0x1a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Concurrent execution of module unload and get_info() trigered the warning.
The root cause is as follows:
cpu0 cpu1
module_exit
//mod->state = MODULE_STATE_GOING
ip6table_nat_exit
xt_unregister_template
//remove table from templ list
getinfo()
t = xt_find_table_lock
list_for_each_entry(tmpl, &xt_templates[af]...)
if (strcmp(tmpl->name, name))
continue; //table not found
try_module_get
list_for_each_entry(t, &xt_net->tables[af]...)
return t; //not get refcnt
module_put(t->me) //uaf
unregister_pernet_subsys
//remove table from xt_net list
While xt_table module was going away and has been removed from
xt_templates list, we couldnt get refcnt of xt_table->me. Skip
the re-traversal of xt_net->tables list to fix it.
Fixes: c22921df777d ("netfilter: iptables: Fix potential null-ptr-deref in ip6table_nat_table_init().")
Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>
---
net/netfilter/x_tables.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index da5d929c7c85..359c880ecb07 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1239,6 +1239,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
struct module *owner = NULL;
struct xt_template *tmpl;
struct xt_table *t;
+ int err = -ENOENT;
mutex_lock(&xt[af].mutex);
list_for_each_entry(t, &xt_net->tables[af], list)
@@ -1247,8 +1248,6 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
/* Table doesn't exist in this netns, check larval list */
list_for_each_entry(tmpl, &xt_templates[af], list) {
- int err;
-
if (strcmp(tmpl->name, name))
continue;
if (!try_module_get(tmpl->me))
@@ -1267,6 +1266,9 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
break;
}
+ if (err < 0)
+ goto out;
+
/* and once again: */
list_for_each_entry(t, &xt_net->tables[af], list)
if (strcmp(t->name, name) == 0)
@@ -1275,7 +1277,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
module_put(owner);
out:
mutex_unlock(&xt[af].mutex);
- return ERR_PTR(-ENOENT);
+ return ERR_PTR(err);
}
EXPORT_SYMBOL_GPL(xt_find_table_lock);
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
2024-10-22 8:57 Dong Chenchen
@ 2024-10-22 9:22 ` Yue Haibing
2024-10-22 9:48 ` Florian Westphal
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Yue Haibing @ 2024-10-22 9:22 UTC (permalink / raw)
To: Dong Chenchen, pablo, kadlec, davem, edumazet, kuba, pabeni, fw,
kuniyu
Cc: netfilter-devel, netdev
On 2024/10/22 16:57, Dong Chenchen wrote:
> ip6table_nat module unload has refcnt warning for UAF. call trace is:
>
> WARNING: CPU: 1 PID: 379 at kernel/module/main.c:853 module_put+0x6f/0x80
> Modules linked in: ip6table_nat(-)
> CPU: 1 UID: 0 PID: 379 Comm: ip6tables Not tainted 6.12.0-rc4-00047-gc2ee9f594da8-dirty #205
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> RIP: 0010:module_put+0x6f/0x80
> Call Trace:
> <TASK>
> get_info+0x128/0x180
> do_ip6t_get_ctl+0x6a/0x430
> nf_getsockopt+0x46/0x80
> ipv6_getsockopt+0xb9/0x100
> rawv6_getsockopt+0x42/0x190
> do_sock_getsockopt+0xaa/0x180
> __sys_getsockopt+0x70/0xc0
> __x64_sys_getsockopt+0x20/0x30
> do_syscall_64+0xa2/0x1a0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Concurrent execution of module unload and get_info() trigered the warning.
> The root cause is as follows:
>
> cpu0 cpu1
> module_exit
> //mod->state = MODULE_STATE_GOING
> ip6table_nat_exit
> xt_unregister_template
> //remove table from templ list
> getinfo()
> t = xt_find_table_lock
> list_for_each_entry(tmpl, &xt_templates[af]...)
> if (strcmp(tmpl->name, name))
> continue; //table not found
> try_module_get
> list_for_each_entry(t, &xt_net->tables[af]...)
> return t; //not get refcnt
> module_put(t->me) //uaf
> unregister_pernet_subsys
> //remove table from xt_net list
>
> While xt_table module was going away and has been removed from
> xt_templates list, we couldnt get refcnt of xt_table->me. Skip
> the re-traversal of xt_net->tables list to fix it.
>
> Fixes: c22921df777d ("netfilter: iptables: Fix potential null-ptr-deref in ip6table_nat_table_init().")
This should be
Fixes: fdacd57c79b7 ("netfilter: x_tables: never register tables by default")
> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>
> ---
> net/netfilter/x_tables.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index da5d929c7c85..359c880ecb07 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -1239,6 +1239,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> struct module *owner = NULL;
> struct xt_template *tmpl;
> struct xt_table *t;
> + int err = -ENOENT;
>
> mutex_lock(&xt[af].mutex);
> list_for_each_entry(t, &xt_net->tables[af], list)
> @@ -1247,8 +1248,6 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>
> /* Table doesn't exist in this netns, check larval list */
> list_for_each_entry(tmpl, &xt_templates[af], list) {
> - int err;
> -
> if (strcmp(tmpl->name, name))
> continue;
> if (!try_module_get(tmpl->me))
> @@ -1267,6 +1266,9 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> break;
> }
>
> + if (err < 0)
> + goto out;
> +
> /* and once again: */
> list_for_each_entry(t, &xt_net->tables[af], list)
> if (strcmp(t->name, name) == 0)
> @@ -1275,7 +1277,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> module_put(owner);
> out:
> mutex_unlock(&xt[af].mutex);
> - return ERR_PTR(-ENOENT);
> + return ERR_PTR(err);
> }
> EXPORT_SYMBOL_GPL(xt_find_table_lock);
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
2024-10-22 8:57 Dong Chenchen
2024-10-22 9:22 ` Yue Haibing
@ 2024-10-22 9:48 ` Florian Westphal
2024-10-23 7:03 ` dongchenchen (A)
2024-10-22 15:33 ` Simon Horman
2024-10-23 17:16 ` Dan Carpenter
3 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2024-10-22 9:48 UTC (permalink / raw)
To: Dong Chenchen
Cc: pablo, kadlec, davem, edumazet, kuba, pabeni, fw, kuniyu,
netfilter-devel, netdev, yuehaibing
Dong Chenchen <dongchenchen2@huawei.com> wrote:
> net/netfilter/x_tables.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index da5d929c7c85..359c880ecb07 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -1239,6 +1239,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> struct module *owner = NULL;
> struct xt_template *tmpl;
> struct xt_table *t;
> + int err = -ENOENT;
>
> mutex_lock(&xt[af].mutex);
> list_for_each_entry(t, &xt_net->tables[af], list)
> @@ -1247,8 +1248,6 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>
> /* Table doesn't exist in this netns, check larval list */
> list_for_each_entry(tmpl, &xt_templates[af], list) {
> - int err;
> -
> if (strcmp(tmpl->name, name))
> continue;
> if (!try_module_get(tmpl->me))
> @@ -1267,6 +1266,9 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> break;
> }
>
> + if (err < 0)
> + goto out;
> +
> /* and once again: */
> list_for_each_entry(t, &xt_net->tables[af], list)
> if (strcmp(t->name, name) == 0)
Proabably also:
- if (strcmp(t->name, name) == 0)
+ if (strcmp(t->name, name) == 0 && owner == t->me)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
2024-10-22 9:48 ` Florian Westphal
@ 2024-10-23 7:03 ` dongchenchen (A)
0 siblings, 0 replies; 11+ messages in thread
From: dongchenchen (A) @ 2024-10-23 7:03 UTC (permalink / raw)
To: Florian Westphal
Cc: pablo, kadlec, davem, edumazet, kuba, pabeni, kuniyu,
netfilter-devel, netdev, yuehaibing
On 2024/10/22 17:48, Florian Westphal wrote:
> Dong Chenchen <dongchenchen2@huawei.com> wrote:
>> net/netfilter/x_tables.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
>> index da5d929c7c85..359c880ecb07 100644
>> --- a/net/netfilter/x_tables.c
>> +++ b/net/netfilter/x_tables.c
>> @@ -1239,6 +1239,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>> struct module *owner = NULL;
>> struct xt_template *tmpl;
>> struct xt_table *t;
>> + int err = -ENOENT;
>>
>> mutex_lock(&xt[af].mutex);
>> list_for_each_entry(t, &xt_net->tables[af], list)
>> @@ -1247,8 +1248,6 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>>
>> /* Table doesn't exist in this netns, check larval list */
>> list_for_each_entry(tmpl, &xt_templates[af], list) {
>> - int err;
>> -
>> if (strcmp(tmpl->name, name))
>> continue;
>> if (!try_module_get(tmpl->me))
>> @@ -1267,6 +1266,9 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>> break;
>> }
>>
>> + if (err < 0)
>> + goto out;
>> +
>> /* and once again: */
>> list_for_each_entry(t, &xt_net->tables[af], list)
>> if (strcmp(t->name, name) == 0)
> Proabably also:
>
> - if (strcmp(t->name, name) == 0)
> + if (strcmp(t->name, name) == 0 && owner == t->me)
Hi, Florian. Thank you very much for your suggestions!
If err <0 , xt_table does not exist in the xt_net list or the tmpl list.
And list operations are performed in the mutex lock.
Therefore, it may not be necessary to traverse the xt_net list again.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
2024-10-22 8:57 Dong Chenchen
2024-10-22 9:22 ` Yue Haibing
2024-10-22 9:48 ` Florian Westphal
@ 2024-10-22 15:33 ` Simon Horman
2024-10-23 6:32 ` dongchenchen (A)
2024-10-23 17:16 ` Dan Carpenter
3 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2024-10-22 15:33 UTC (permalink / raw)
To: Dong Chenchen
Cc: pablo, kadlec, davem, edumazet, kuba, pabeni, fw, kuniyu,
netfilter-devel, netdev, yuehaibing
On Tue, Oct 22, 2024 at 04:57:53PM +0800, Dong Chenchen wrote:
> ip6table_nat module unload has refcnt warning for UAF. call trace is:
>
> WARNING: CPU: 1 PID: 379 at kernel/module/main.c:853 module_put+0x6f/0x80
> Modules linked in: ip6table_nat(-)
> CPU: 1 UID: 0 PID: 379 Comm: ip6tables Not tainted 6.12.0-rc4-00047-gc2ee9f594da8-dirty #205
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> RIP: 0010:module_put+0x6f/0x80
> Call Trace:
> <TASK>
> get_info+0x128/0x180
> do_ip6t_get_ctl+0x6a/0x430
> nf_getsockopt+0x46/0x80
> ipv6_getsockopt+0xb9/0x100
> rawv6_getsockopt+0x42/0x190
> do_sock_getsockopt+0xaa/0x180
> __sys_getsockopt+0x70/0xc0
> __x64_sys_getsockopt+0x20/0x30
> do_syscall_64+0xa2/0x1a0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Concurrent execution of module unload and get_info() trigered the warning.
> The root cause is as follows:
>
> cpu0 cpu1
> module_exit
> //mod->state = MODULE_STATE_GOING
> ip6table_nat_exit
> xt_unregister_template
> //remove table from templ list
> getinfo()
> t = xt_find_table_lock
> list_for_each_entry(tmpl, &xt_templates[af]...)
> if (strcmp(tmpl->name, name))
> continue; //table not found
> try_module_get
> list_for_each_entry(t, &xt_net->tables[af]...)
> return t; //not get refcnt
> module_put(t->me) //uaf
> unregister_pernet_subsys
> //remove table from xt_net list
>
> While xt_table module was going away and has been removed from
> xt_templates list, we couldnt get refcnt of xt_table->me. Skip
> the re-traversal of xt_net->tables list to fix it.
>
> Fixes: c22921df777d ("netfilter: iptables: Fix potential null-ptr-deref in ip6table_nat_table_init().")
> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>
> ---
> net/netfilter/x_tables.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index da5d929c7c85..359c880ecb07 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -1239,6 +1239,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> struct module *owner = NULL;
> struct xt_template *tmpl;
> struct xt_table *t;
> + int err = -ENOENT;
>
> mutex_lock(&xt[af].mutex);
> list_for_each_entry(t, &xt_net->tables[af], list)
> @@ -1247,8 +1248,6 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>
> /* Table doesn't exist in this netns, check larval list */
> list_for_each_entry(tmpl, &xt_templates[af], list) {
> - int err;
> -
> if (strcmp(tmpl->name, name))
> continue;
> if (!try_module_get(tmpl->me))
> @@ -1267,6 +1266,9 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> break;
> }
>
> + if (err < 0)
> + goto out;
> +
> /* and once again: */
> list_for_each_entry(t, &xt_net->tables[af], list)
> if (strcmp(t->name, name) == 0)
> @@ -1275,7 +1277,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> module_put(owner);
Hi Dong Chenchen,
I'm unsure if this can happen in practice, although I guess so else the
module_put() call above is never reached. In any case, previously if we got
to this line then the function would return ERR_PTR(-ENOENT). But now it
will return ERR_PTR(0). Which although valid often indicates a bug.
Flagged by Smatch.
> out:
> mutex_unlock(&xt[af].mutex);
> - return ERR_PTR(-ENOENT);
> + return ERR_PTR(err);
> }
> EXPORT_SYMBOL_GPL(xt_find_table_lock);
>
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
2024-10-22 15:33 ` Simon Horman
@ 2024-10-23 6:32 ` dongchenchen (A)
2024-10-23 6:52 ` dongchenchen (A)
0 siblings, 1 reply; 11+ messages in thread
From: dongchenchen (A) @ 2024-10-23 6:32 UTC (permalink / raw)
To: Simon Horman
Cc: pablo, kadlec, davem, edumazet, kuba, pabeni, fw, kuniyu,
netfilter-devel, netdev, yuehaibing
On 2024/10/22 23:33, Simon Horman wrote:
> On Tue, Oct 22, 2024 at 04:57:53PM +0800, Dong Chenchen wrote:
>> ip6table_nat module unload has refcnt warning for UAF. call trace is:
>>
>> WARNING: CPU: 1 PID: 379 at kernel/module/main.c:853 module_put+0x6f/0x80
>> Modules linked in: ip6table_nat(-)
>> CPU: 1 UID: 0 PID: 379 Comm: ip6tables Not tainted 6.12.0-rc4-00047-gc2ee9f594da8-dirty #205
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>> BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
>> RIP: 0010:module_put+0x6f/0x80
>> Call Trace:
>> <TASK>
>> get_info+0x128/0x180
>> do_ip6t_get_ctl+0x6a/0x430
>> nf_getsockopt+0x46/0x80
>> ipv6_getsockopt+0xb9/0x100
>> rawv6_getsockopt+0x42/0x190
>> do_sock_getsockopt+0xaa/0x180
>> __sys_getsockopt+0x70/0xc0
>> __x64_sys_getsockopt+0x20/0x30
>> do_syscall_64+0xa2/0x1a0
>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>>
>> Concurrent execution of module unload and get_info() trigered the warning.
>> The root cause is as follows:
>>
>> cpu0 cpu1
>> module_exit
>> //mod->state = MODULE_STATE_GOING
>> ip6table_nat_exit
>> xt_unregister_template
>> //remove table from templ list
>> getinfo()
>> t = xt_find_table_lock
>> list_for_each_entry(tmpl, &xt_templates[af]...)
>> if (strcmp(tmpl->name, name))
>> continue; //table not found
>> try_module_get
>> list_for_each_entry(t, &xt_net->tables[af]...)
>> return t; //not get refcnt
>> module_put(t->me) //uaf
>> unregister_pernet_subsys
>> //remove table from xt_net list
>>
>> While xt_table module was going away and has been removed from
>> xt_templates list, we couldnt get refcnt of xt_table->me. Skip
>> the re-traversal of xt_net->tables list to fix it.
>>
>> Fixes: c22921df777d ("netfilter: iptables: Fix potential null-ptr-deref in ip6table_nat_table_init().")
>> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>
>> ---
>> net/netfilter/x_tables.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
>> index da5d929c7c85..359c880ecb07 100644
>> --- a/net/netfilter/x_tables.c
>> +++ b/net/netfilter/x_tables.c
>> @@ -1239,6 +1239,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>> struct module *owner = NULL;
>> struct xt_template *tmpl;
>> struct xt_table *t;
>> + int err = -ENOENT;
>>
>> mutex_lock(&xt[af].mutex);
>> list_for_each_entry(t, &xt_net->tables[af], list)
>> @@ -1247,8 +1248,6 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>>
>> /* Table doesn't exist in this netns, check larval list */
>> list_for_each_entry(tmpl, &xt_templates[af], list) {
>> - int err;
>> -
>> if (strcmp(tmpl->name, name))
>> continue;
>> if (!try_module_get(tmpl->me))
>> @@ -1267,6 +1266,9 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>> break;
>> }
>>
>> + if (err < 0)
>> + goto out;
>> +
>> /* and once again: */
>> list_for_each_entry(t, &xt_net->tables[af], list)
>> if (strcmp(t->name, name) == 0)
>> @@ -1275,7 +1277,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
>> module_put(owner);
> Hi Dong Chenchen,
>
> I'm unsure if this can happen in practice, although I guess so else the
> module_put() call above is never reached.
Hi, Simon. Thank you very much for your suggestions!
module_put(owner) will be never reached indeed. which wiil be executed in:
1. xt_table not found in tmpl list and xt_net list:
Owner == NULL, no need to put
2. xt_table found in tmpl list; table_init() fail to add table to
xt_net list but return 0
this situation may be mutually exclusive
So I thought it may not need to call module_puy(owner) here
xt_find_table_lock
list_for_each_entry(tmpl, &xt_templates[af], list)
if (strcmp(tmpl->name, name))
continue;
err = tmpl->table_init(net); //add xtable to xt_net list
if (err < 0) {
module_put(owner);
return ERR_PTR(err);
}
list_for_each_entry(t, &xt_net->tables[af], list)
if (strcmp(t->name, name) == 0)
return t;//err = 0, will return here
module_put(owner); // put effectively while (err == 0) && (xtable found
in tmpl list) and add table xt_net list failed in table_init()
out:
mutex_unlock(&xt[af].mutex);
return ERR_PTR(-ENOENT);
> In any case, previously if we got
> to this line then the function would return ERR_PTR(-ENOENT). But now it
> will return ERR_PTR(0). Which although valid often indicates a bug.
>
> Flagged by Smatch.
As described above, err = 0 will be return in xt_net table list re-
traversal.
>> out:
>> mutex_unlock(&xt[af].mutex);
>> - return ERR_PTR(-ENOENT);
>> + return ERR_PTR(err);
>> }
>> EXPORT_SYMBOL_GPL(xt_find_table_lock);
>>
>> --
>> 2.25.1
>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
2024-10-23 6:32 ` dongchenchen (A)
@ 2024-10-23 6:52 ` dongchenchen (A)
0 siblings, 0 replies; 11+ messages in thread
From: dongchenchen (A) @ 2024-10-23 6:52 UTC (permalink / raw)
To: dongchenchen2
Cc: pablo, kadlec, davem, edumazet, kuba, pabeni, fw, kuniyu,
netfilter-devel, netdev, yuehaibing, Simon Horman
On 2024/10/23 14:32, dongchenchen (A) wrote:
>
> On 2024/10/22 23:33, Simon Horman wrote:
>> On Tue, Oct 22, 2024 at 04:57:53PM +0800, Dong Chenchen wrote:
>>> ip6table_nat module unload has refcnt warning for UAF. call trace is:
>>>
>>> WARNING: CPU: 1 PID: 379 at kernel/module/main.c:853
>>> module_put+0x6f/0x80
>>> Modules linked in: ip6table_nat(-)
>>> CPU: 1 UID: 0 PID: 379 Comm: ip6tables Not tainted
>>> 6.12.0-rc4-00047-gc2ee9f594da8-dirty #205
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>>> BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
>>> RIP: 0010:module_put+0x6f/0x80
>>> Call Trace:
>>> <TASK>
>>> get_info+0x128/0x180
>>> do_ip6t_get_ctl+0x6a/0x430
>>> nf_getsockopt+0x46/0x80
>>> ipv6_getsockopt+0xb9/0x100
>>> rawv6_getsockopt+0x42/0x190
>>> do_sock_getsockopt+0xaa/0x180
>>> __sys_getsockopt+0x70/0xc0
>>> __x64_sys_getsockopt+0x20/0x30
>>> do_syscall_64+0xa2/0x1a0
>>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>>>
>>> Concurrent execution of module unload and get_info() trigered the
>>> warning.
>>> The root cause is as follows:
>>>
>>> cpu0 cpu1
>>> module_exit
>>> //mod->state = MODULE_STATE_GOING
>>> ip6table_nat_exit
>>> xt_unregister_template
>>> //remove table from templ list
>>> getinfo()
>>> t = xt_find_table_lock
>>> list_for_each_entry(tmpl, &xt_templates[af]...)
>>> if (strcmp(tmpl->name, name))
>>> continue; //table not found
>>> try_module_get
>>> list_for_each_entry(t, &xt_net->tables[af]...)
>>> return t; //not get refcnt
>>> module_put(t->me) //uaf
>>> unregister_pernet_subsys
>>> //remove table from xt_net list
>>>
>>> While xt_table module was going away and has been removed from
>>> xt_templates list, we couldnt get refcnt of xt_table->me. Skip
>>> the re-traversal of xt_net->tables list to fix it.
>>>
>>> Fixes: c22921df777d ("netfilter: iptables: Fix potential
>>> null-ptr-deref in ip6table_nat_table_init().")
>>> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com>
>>> ---
>>> net/netfilter/x_tables.c | 8 +++++---
>>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
>>> index da5d929c7c85..359c880ecb07 100644
>>> --- a/net/netfilter/x_tables.c
>>> +++ b/net/netfilter/x_tables.c
>>> @@ -1239,6 +1239,7 @@ struct xt_table *xt_find_table_lock(struct net
>>> *net, u_int8_t af,
>>> struct module *owner = NULL;
>>> struct xt_template *tmpl;
>>> struct xt_table *t;
>>> + int err = -ENOENT;
>>> mutex_lock(&xt[af].mutex);
>>> list_for_each_entry(t, &xt_net->tables[af], list)
>>> @@ -1247,8 +1248,6 @@ struct xt_table *xt_find_table_lock(struct net
>>> *net, u_int8_t af,
>>> /* Table doesn't exist in this netns, check larval list */
>>> list_for_each_entry(tmpl, &xt_templates[af], list) {
>>> - int err;
>>> -
>>> if (strcmp(tmpl->name, name))
>>> continue;
>>> if (!try_module_get(tmpl->me))
>>> @@ -1267,6 +1266,9 @@ struct xt_table *xt_find_table_lock(struct net
>>> *net, u_int8_t af,
>>> break;
>>> }
>>> + if (err < 0)
>>> + goto out;
>>> +
>>> /* and once again: */
>>> list_for_each_entry(t, &xt_net->tables[af], list)
>>> if (strcmp(t->name, name) == 0)
>>> @@ -1275,7 +1277,7 @@ struct xt_table *xt_find_table_lock(struct net
>>> *net, u_int8_t af,
>>> module_put(owner);
>> Hi Dong Chenchen,
>>
>> I'm unsure if this can happen in practice, although I guess so else the
>> module_put() call above is never reached.
>
> Hi, Simon. Thank you very much for your suggestions!
>
> module_put(owner) will be never reached indeed. which wiil be executed
> in:
>
>
sorry, there is a problem with the email format. resend:
module_put(owner) wiil be executed in:
1. xt_table not found in tmpl list and xt_net list:
owner == NULL, no need to put
2. xt_table found in tmpl list, table_init() fail to add table to xt_net
list but return 0
this situation may be mutually exclusive
So I thought it may not need to call module_puy(owner) here
xt_find_table_lock
list_for_each_entry(tmpl, &xt_templates[af], list)
if (strcmp(tmpl->name, name))
continue;
err = tmpl->table_init(net); //add xtable to xt_net list
if (err < 0) {
module_put(owner);
return ERR_PTR(err);
}
list_for_each_entry(t, &xt_net->tables[af], list)
if (strcmp(t->name, name) == 0)
return t; //err = 0, will return here
module_put(owner); // put effectively while (err == 0) && (xtable
found in tmpl list) and add table xt_net list failed in table_init()
out:
mutex_unlock(&xt[af].mutex);
return ERR_PTR(-ENOENT);
>> In any case, previously if we got
>> to this line then the function would return ERR_PTR(-ENOENT). But now it
>> will return ERR_PTR(0). Which although valid often indicates a bug.
>>
>> Flagged by Smatch.
>
> As described above, err = 0 will be return in xt_net table list re-
> traversal.
>
>>> out:
>>> mutex_unlock(&xt[af].mutex);
>>> - return ERR_PTR(-ENOENT);
>>> + return ERR_PTR(err);
>>> }
>>> EXPORT_SYMBOL_GPL(xt_find_table_lock);
>>> --
>>> 2.25.1
>>>
>>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
2024-10-22 8:57 Dong Chenchen
` (2 preceding siblings ...)
2024-10-22 15:33 ` Simon Horman
@ 2024-10-23 17:16 ` Dan Carpenter
2024-10-24 1:57 ` dongchenchen (A)
3 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2024-10-23 17:16 UTC (permalink / raw)
To: oe-kbuild, Dong Chenchen, pablo, kadlec, davem, edumazet, kuba,
pabeni, fw, kuniyu
Cc: lkp, oe-kbuild-all, netfilter-devel, netdev, yuehaibing,
Dong Chenchen
Hi Dong,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Dong-Chenchen/net-netfilter-Fix-use-after-free-in-get_info/20241022-165936
base: net/main
patch link: https://lore.kernel.org/r/20241022085753.2069639-1-dongchenchen2%40huawei.com
patch subject: [PATCH net] net: netfilter: Fix use-after-free in get_info()
config: x86_64-randconfig-161-20241023 (https://download.01.org/0day-ci/archive/20241024/202410240020.Cqi2d68p-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202410240020.Cqi2d68p-lkp@intel.com/
smatch warnings:
net/netfilter/x_tables.c:1280 xt_find_table_lock() warn: passing zero to 'ERR_PTR'
vim +/ERR_PTR +1280 net/netfilter/x_tables.c
03d13b6868a261 Florian Westphal 2017-12-08 1234 /* Find table by name, grabs mutex & ref. Returns ERR_PTR on error. */
76108cea065cda Jan Engelhardt 2008-10-08 1235 struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
76108cea065cda Jan Engelhardt 2008-10-08 1236 const char *name)
2e4e6a17af35be Harald Welte 2006-01-12 1237 {
1d610d4d31a8ed Florian Westphal 2021-04-01 1238 struct xt_pernet *xt_net = net_generic(net, xt_pernet_id);
fdacd57c79b79a Florian Westphal 2021-08-03 1239 struct module *owner = NULL;
fdacd57c79b79a Florian Westphal 2021-08-03 1240 struct xt_template *tmpl;
fdacd57c79b79a Florian Westphal 2021-08-03 1241 struct xt_table *t;
f4f502d5a8ea29 Dong Chenchen 2024-10-22 1242 int err = -ENOENT;
2e4e6a17af35be Harald Welte 2006-01-12 1243
7926dbfa4bc14e Pablo Neira Ayuso 2014-07-31 1244 mutex_lock(&xt[af].mutex);
1d610d4d31a8ed Florian Westphal 2021-04-01 1245 list_for_each_entry(t, &xt_net->tables[af], list)
2e4e6a17af35be Harald Welte 2006-01-12 1246 if (strcmp(t->name, name) == 0 && try_module_get(t->me))
2e4e6a17af35be Harald Welte 2006-01-12 1247 return t;
b9e69e12739718 Florian Westphal 2016-02-25 1248
fdacd57c79b79a Florian Westphal 2021-08-03 1249 /* Table doesn't exist in this netns, check larval list */
fdacd57c79b79a Florian Westphal 2021-08-03 1250 list_for_each_entry(tmpl, &xt_templates[af], list) {
fdacd57c79b79a Florian Westphal 2021-08-03 1251 if (strcmp(tmpl->name, name))
b9e69e12739718 Florian Westphal 2016-02-25 1252 continue;
fdacd57c79b79a Florian Westphal 2021-08-03 1253 if (!try_module_get(tmpl->me))
03d13b6868a261 Florian Westphal 2017-12-08 1254 goto out;
fdacd57c79b79a Florian Westphal 2021-08-03 1255
fdacd57c79b79a Florian Westphal 2021-08-03 1256 owner = tmpl->me;
fdacd57c79b79a Florian Westphal 2021-08-03 1257
b9e69e12739718 Florian Westphal 2016-02-25 1258 mutex_unlock(&xt[af].mutex);
fdacd57c79b79a Florian Westphal 2021-08-03 1259 err = tmpl->table_init(net);
03d13b6868a261 Florian Westphal 2017-12-08 1260 if (err < 0) {
fdacd57c79b79a Florian Westphal 2021-08-03 1261 module_put(owner);
03d13b6868a261 Florian Westphal 2017-12-08 1262 return ERR_PTR(err);
b9e69e12739718 Florian Westphal 2016-02-25 1263 }
b9e69e12739718 Florian Westphal 2016-02-25 1264
b9e69e12739718 Florian Westphal 2016-02-25 1265 mutex_lock(&xt[af].mutex);
b9e69e12739718 Florian Westphal 2016-02-25 1266 break;
b9e69e12739718 Florian Westphal 2016-02-25 1267 }
b9e69e12739718 Florian Westphal 2016-02-25 1268
f4f502d5a8ea29 Dong Chenchen 2024-10-22 1269 if (err < 0)
f4f502d5a8ea29 Dong Chenchen 2024-10-22 1270 goto out;
f4f502d5a8ea29 Dong Chenchen 2024-10-22 1271
b9e69e12739718 Florian Westphal 2016-02-25 1272 /* and once again: */
1d610d4d31a8ed Florian Westphal 2021-04-01 1273 list_for_each_entry(t, &xt_net->tables[af], list)
b9e69e12739718 Florian Westphal 2016-02-25 1274 if (strcmp(t->name, name) == 0)
b9e69e12739718 Florian Westphal 2016-02-25 1275 return t;
ret it zero here, but if we fail to find the name then we should set ret =
-ENOENT;
b9e69e12739718 Florian Westphal 2016-02-25 1276
fdacd57c79b79a Florian Westphal 2021-08-03 1277 module_put(owner);
b9e69e12739718 Florian Westphal 2016-02-25 1278 out:
9e19bb6d7a0959 Ingo Molnar 2006-03-25 1279 mutex_unlock(&xt[af].mutex);
f4f502d5a8ea29 Dong Chenchen 2024-10-22 @1280 return ERR_PTR(err);
2e4e6a17af35be Harald Welte 2006-01-12 1281 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] net: netfilter: Fix use-after-free in get_info()
2024-10-23 17:16 ` Dan Carpenter
@ 2024-10-24 1:57 ` dongchenchen (A)
0 siblings, 0 replies; 11+ messages in thread
From: dongchenchen (A) @ 2024-10-24 1:57 UTC (permalink / raw)
To: Dan Carpenter, oe-kbuild, pablo, kadlec, davem, edumazet, kuba,
pabeni, fw, kuniyu
Cc: lkp, oe-kbuild-all, netfilter-devel, netdev, yuehaibing
On 2024/10/24 1:16, Dan Carpenter wrote:
> Hi Dong,
>
> kernel test robot noticed the following build warnings:
>
> url: https://github.com/intel-lab-lkp/linux/commits/Dong-Chenchen/net-netfilter-Fix-use-after-free-in-get_info/20241022-165936
> base: net/main
> patch link: https://lore.kernel.org/r/20241022085753.2069639-1-dongchenchen2%40huawei.com
> patch subject: [PATCH net] net: netfilter: Fix use-after-free in get_info()
> config: x86_64-randconfig-161-20241023 (https://download.01.org/0day-ci/archive/20241024/202410240020.Cqi2d68p-lkp@intel.com/config)
> compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202410240020.Cqi2d68p-lkp@intel.com/
>
> smatch warnings:
> net/netfilter/x_tables.c:1280 xt_find_table_lock() warn: passing zero to 'ERR_PTR'
>
> vim +/ERR_PTR +1280 net/netfilter/x_tables.c
>
> 03d13b6868a261 Florian Westphal 2017-12-08 1234 /* Find table by name, grabs mutex & ref. Returns ERR_PTR on error. */
> 76108cea065cda Jan Engelhardt 2008-10-08 1235 struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
> 76108cea065cda Jan Engelhardt 2008-10-08 1236 const char *name)
> 2e4e6a17af35be Harald Welte 2006-01-12 1237 {
> 1d610d4d31a8ed Florian Westphal 2021-04-01 1238 struct xt_pernet *xt_net = net_generic(net, xt_pernet_id);
> fdacd57c79b79a Florian Westphal 2021-08-03 1239 struct module *owner = NULL;
> fdacd57c79b79a Florian Westphal 2021-08-03 1240 struct xt_template *tmpl;
> fdacd57c79b79a Florian Westphal 2021-08-03 1241 struct xt_table *t;
> f4f502d5a8ea29 Dong Chenchen 2024-10-22 1242 int err = -ENOENT;
> 2e4e6a17af35be Harald Welte 2006-01-12 1243
> 7926dbfa4bc14e Pablo Neira Ayuso 2014-07-31 1244 mutex_lock(&xt[af].mutex);
> 1d610d4d31a8ed Florian Westphal 2021-04-01 1245 list_for_each_entry(t, &xt_net->tables[af], list)
> 2e4e6a17af35be Harald Welte 2006-01-12 1246 if (strcmp(t->name, name) == 0 && try_module_get(t->me))
> 2e4e6a17af35be Harald Welte 2006-01-12 1247 return t;
> b9e69e12739718 Florian Westphal 2016-02-25 1248
> fdacd57c79b79a Florian Westphal 2021-08-03 1249 /* Table doesn't exist in this netns, check larval list */
> fdacd57c79b79a Florian Westphal 2021-08-03 1250 list_for_each_entry(tmpl, &xt_templates[af], list) {
> fdacd57c79b79a Florian Westphal 2021-08-03 1251 if (strcmp(tmpl->name, name))
> b9e69e12739718 Florian Westphal 2016-02-25 1252 continue;
> fdacd57c79b79a Florian Westphal 2021-08-03 1253 if (!try_module_get(tmpl->me))
> 03d13b6868a261 Florian Westphal 2017-12-08 1254 goto out;
> fdacd57c79b79a Florian Westphal 2021-08-03 1255
> fdacd57c79b79a Florian Westphal 2021-08-03 1256 owner = tmpl->me;
> fdacd57c79b79a Florian Westphal 2021-08-03 1257
> b9e69e12739718 Florian Westphal 2016-02-25 1258 mutex_unlock(&xt[af].mutex);
> fdacd57c79b79a Florian Westphal 2021-08-03 1259 err = tmpl->table_init(net);
> 03d13b6868a261 Florian Westphal 2017-12-08 1260 if (err < 0) {
> fdacd57c79b79a Florian Westphal 2021-08-03 1261 module_put(owner);
> 03d13b6868a261 Florian Westphal 2017-12-08 1262 return ERR_PTR(err);
> b9e69e12739718 Florian Westphal 2016-02-25 1263 }
> b9e69e12739718 Florian Westphal 2016-02-25 1264
If rmmod is executed concurrently here, xtable will be remove from
xt_net list，
which may lead to ERR_PTR(0).
Thank you for your review. v2 has been sent
> b9e69e12739718 Florian Westphal 2016-02-25 1265 mutex_lock(&xt[af].mutex);
> b9e69e12739718 Florian Westphal 2016-02-25 1266 break;
> b9e69e12739718 Florian Westphal 2016-02-25 1267 }
> b9e69e12739718 Florian Westphal 2016-02-25 1268
> f4f502d5a8ea29 Dong Chenchen 2024-10-22 1269 if (err < 0)
> f4f502d5a8ea29 Dong Chenchen 2024-10-22 1270 goto out;
> f4f502d5a8ea29 Dong Chenchen 2024-10-22 1271
> b9e69e12739718 Florian Westphal 2016-02-25 1272 /* and once again: */
> 1d610d4d31a8ed Florian Westphal 2021-04-01 1273 list_for_each_entry(t, &xt_net->tables[af], list)
> b9e69e12739718 Florian Westphal 2016-02-25 1274 if (strcmp(t->name, name) == 0)
> b9e69e12739718 Florian Westphal 2016-02-25 1275 return t;
>
> ret it zero here, but if we fail to find the name then we should set ret =
> -ENOENT;
>
> b9e69e12739718 Florian Westphal 2016-02-25 1276
> fdacd57c79b79a Florian Westphal 2021-08-03 1277 module_put(owner);
> b9e69e12739718 Florian Westphal 2016-02-25 1278 out:
> 9e19bb6d7a0959 Ingo Molnar 2006-03-25 1279 mutex_unlock(&xt[af].mutex);
> f4f502d5a8ea29 Dong Chenchen 2024-10-22 @1280 return ERR_PTR(err);
> 2e4e6a17af35be Harald Welte 2006-01-12 1281 }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-10-24 1:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-23 17:07 [PATCH net] net: netfilter: Fix use-after-free in get_info() kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2024-10-23 1:28 dongchenchen (A)
2024-10-22 8:57 Dong Chenchen
2024-10-22 9:22 ` Yue Haibing
2024-10-22 9:48 ` Florian Westphal
2024-10-23 7:03 ` dongchenchen (A)
2024-10-22 15:33 ` Simon Horman
2024-10-23 6:32 ` dongchenchen (A)
2024-10-23 6:52 ` dongchenchen (A)
2024-10-23 17:16 ` Dan Carpenter
2024-10-24 1:57 ` dongchenchen (A)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.