lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANn89iJdZx=e2QN_AXPiZQDh4u4EY5dOrzgdsqgWTCpvLhJVcQ@mail.gmail.com> Date: Mon, 17 Oct 2022 08:58:17 -0700 From: Eric Dumazet <edumazet@...gle.com> To: Zhengchao Shao <shaozhengchao@...wei.com> Cc: netdev@...r.kernel.org, davem@...emloft.net, yoshfuji@...ux-ipv6.org, dsahern@...nel.org, kuba@...nel.org, pabeni@...hat.com, weiyongjun1@...wei.com, yuehaibing@...wei.com Subject: Re: [PATCH net] ip6mr: fix UAF issue in ip6mr_sk_done() when addrconf_init_net() failed On Mon, Oct 17, 2022 at 12:55 AM Zhengchao Shao <shaozhengchao@...wei.com> wrote: > > If the initialization fails in calling addrconf_init_net(), devconf_all is > the pointer that has been released. Then ip6mr_sk_done() is called to > release the net, accessing devconf->mc_forwarding directly causes invalid > pointer access. > > The process is as follows: > setup_net() > ops_init() > addrconf_init_net() > all = kmemdup(...) ---> alloc "all" > ... > net->ipv6.devconf_all = all; > __addrconf_sysctl_register() ---> failed > ... > kfree(all); ---> ipv6.devconf_all invalid > ... > ops_exit_list() > ... > ip6mr_sk_done() > devconf = net->ipv6.devconf_all; > //devconf is invalid pointer > if (!devconf || !atomic_read(&devconf->mc_forwarding)) > > > Fixes: 7d9b1b578d67 ("ip6mr: fix use-after-free in ip6mr_sk_done()") Hmmm. I wonder if you are not masking a more serious bug. When I wrote this patch ( 7d9b1b578d67) I was following the standard rule of ops->exit() being called only if the corresponding ops->init() function had not failed. net/core/net_namespace.c:setup_net() even has some comments about this rule: out_undo: /* Walk through the list backwards calling the exit functions * for the pernet modules whose init functions did not fail. */ list_add(&net->exit_list, &net_exit_list); saved_ops = ops; list_for_each_entry_continue_reverse(ops, &pernet_list, list) ops_pre_exit_list(ops, &net_exit_list); synchronize_rcu(); ops = saved_ops; list_for_each_entry_continue_reverse(ops, &pernet_list, list) ops_exit_list(ops, &net_exit_list); ops = saved_ops; list_for_each_entry_continue_reverse(ops, &pernet_list, list) ops_free_list(ops, &net_exit_list); rcu_barrier(); goto out; > Signed-off-by: Zhengchao Shao <shaozhengchao@...wei.com> > --- > net/ipv6/addrconf.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index 417834b7169d..9c3f5202a97b 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -7214,9 +7214,11 @@ static int __net_init addrconf_init_net(struct net *net) > __addrconf_sysctl_unregister(net, all, NETCONFA_IFINDEX_ALL); > err_reg_all: > kfree(dflt); > + net->ipv6.devconf_dflt = NULL; > #endif > err_alloc_dflt: > kfree(all); > + net->ipv6.devconf_all = NULL; > err_alloc_all: > kfree(net->ipv6.inet6_addr_lst); > err_alloc_addr: > -- > 2.17.1 >
Powered by blists - more mailing lists