| Message ID | 536B34FD.8030601@huawei.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, 2014-05-08 at 15:40 +0800, Wang Weidong wrote: > In ipv4_sysctl_init_net, we don't kmemdup a sysctl_table for init_net, > so init_net->ipv4.ipv4_hdr->ctl_table_arg points to ipv4_net_table which > is a static array pointer. So when do ipv4_sysctl_exit_net, it will > free the ipv4_net_table for init_net. > > So add a check net_namespace init_net before kfree the sysctl_table. > > Signed-off-by: Wang Weidong <wangweidong1@huawei.com> > --- > net/ipv4/sysctl_net_ipv4.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c > index 44eba05..2825577 100644 > --- a/net/ipv4/sysctl_net_ipv4.c > +++ b/net/ipv4/sysctl_net_ipv4.c > @@ -891,7 +891,8 @@ static __net_exit void ipv4_sysctl_exit_net(struct net *net) > > table = net->ipv4.ipv4_hdr->ctl_table_arg; > unregister_net_sysctl_table(net->ipv4.ipv4_hdr); > - kfree(table); > + if (!net_eq(net, &init_net)) > + kfree(table); > } > > static __net_initdata struct pernet_operations ipv4_sysctl_ops = { Could you explain how you can trigger this case, calling ipv4_sysctl_exit_net() with net == &init_net ? This would be a bug, your patch would try to hide it maybe ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/5/8 20:34, Eric Dumazet wrote: > On Thu, 2014-05-08 at 15:40 +0800, Wang Weidong wrote: >> In ipv4_sysctl_init_net, we don't kmemdup a sysctl_table for init_net, >> so init_net->ipv4.ipv4_hdr->ctl_table_arg points to ipv4_net_table which >> is a static array pointer. So when do ipv4_sysctl_exit_net, it will >> free the ipv4_net_table for init_net. >> >> So add a check net_namespace init_net before kfree the sysctl_table. >> >> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> >> --- >> net/ipv4/sysctl_net_ipv4.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c >> index 44eba05..2825577 100644 >> --- a/net/ipv4/sysctl_net_ipv4.c >> +++ b/net/ipv4/sysctl_net_ipv4.c >> @@ -891,7 +891,8 @@ static __net_exit void ipv4_sysctl_exit_net(struct net *net) >> >> table = net->ipv4.ipv4_hdr->ctl_table_arg; >> unregister_net_sysctl_table(net->ipv4.ipv4_hdr); >> - kfree(table); >> + if (!net_eq(net, &init_net)) >> + kfree(table); >> } >> >> static __net_initdata struct pernet_operations ipv4_sysctl_ops = { > > Could you explain how you can trigger this case, calling > ipv4_sysctl_exit_net() with net == &init_net ? > > This would be a bug, your patch would try to hide it maybe ? > No. I just trigger the similar case on sctp when I do 'rmmod -f sctp'. Here I add the init_net case for sctp register sysctl. Is it better to add BUG_ON(net == &init_net) maybe? Regards Wang > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> No. > I just trigger the similar case on sctp when I do 'rmmod -f sctp'. > Here I add the init_net case for sctp register sysctl. > > Is it better to add BUG_ON(net == &init_net) maybe? > The point is : SCTP _can_ be a module, but IPV4 is not. You cannot rmmod ipv4 Its not because there is a bug in SCTP that you can apply the same recipe on another layer. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/5/9 2:20, Eric Dumazet wrote: >> No. >> I just trigger the similar case on sctp when I do 'rmmod -f sctp'. >> Here I add the init_net case for sctp register sysctl. >> >> Is it better to add BUG_ON(net == &init_net) maybe? >> > > The point is : SCTP _can_ be a module, but IPV4 is not. > > You cannot rmmod ipv4 > > Its not because there is a bug in SCTP that you can apply the same > recipe on another layer. > Well, You are right. So ignore it. Regards Wang > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index 44eba05..2825577 100644 --- a/net/ipv4/sysctl_net_ipv4.c +++ b/net/ipv4/sysctl_net_ipv4.c @@ -891,7 +891,8 @@ static __net_exit void ipv4_sysctl_exit_net(struct net *net) table = net->ipv4.ipv4_hdr->ctl_table_arg; unregister_net_sysctl_table(net->ipv4.ipv4_hdr); - kfree(table); + if (!net_eq(net, &init_net)) + kfree(table); } static __net_initdata struct pernet_operations ipv4_sysctl_ops = {
In ipv4_sysctl_init_net, we don't kmemdup a sysctl_table for init_net, so init_net->ipv4.ipv4_hdr->ctl_table_arg points to ipv4_net_table which is a static array pointer. So when do ipv4_sysctl_exit_net, it will free the ipv4_net_table for init_net. So add a check net_namespace init_net before kfree the sysctl_table. Signed-off-by: Wang Weidong <wangweidong1@huawei.com> --- net/ipv4/sysctl_net_ipv4.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)