* Re: [Patch-next] Fix the size overflow of addrconf_sysctl array [not found] <4ACEA207.7010208@np.css.fujitsu.com> @ 2009-10-09 5:44 ` David Miller 2009-10-09 13:11 ` Cosmin Ratiu 0 siblings, 1 reply; 4+ messages in thread From: David Miller @ 2009-10-09 5:44 UTC (permalink / raw) To: jin.dongming Cc: linux-kernel, cratiu, opurdila, kaneshige.kenji, seto.hidetoshi, netdev From: Jin Dongming <jin.dongming@np.css.fujitsu.com> Date: Fri, 09 Oct 2009 11:37:59 +0900 Please post networking patches always CC:'d to netdev@vger.kernel.org so that it gets added to our networking patch tracking system at: http://patchwork.ozlabs.org/project/netdev/list/ Thank you. I've applied your fix, thanks! > (This patch fixes bug of commit f7734fdf61ec6bb848e0bafc1fb8bad2c124bb50 > title "make TLLAO option for NA packets configurable") > > When the IPV6 conf is used, the function sysctl_set_parent is called and the > array addrconf_sysctl is used as a parameter of the function. > > The above patch added new conf "force_tllao" into the array addrconf_sysctl, > but the size of the array was not modified, the static allocated size is > DEVCONF_MAX + 1 but the real size is DEVCONF_MAX + 2, so the problem is > that the function sysctl_set_parent accessed wrong address. > > I got the following information. > Call Trace: > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > [<ffffffff810622d5>] __register_sysctl_paths+0xde/0x272 > [<ffffffff8110892d>] ? __kmalloc_track_caller+0x16e/0x180 > [<ffffffffa00cfac3>] ? __addrconf_sysctl_register+0xc5/0x144 [ipv6] > [<ffffffff8141f2c9>] register_net_sysctl_table+0x48/0x4b > [<ffffffffa00cfaf5>] __addrconf_sysctl_register+0xf7/0x144 [ipv6] > [<ffffffffa00cfc16>] addrconf_init_net+0xd4/0x104 [ipv6] > [<ffffffff8139195f>] setup_net+0x35/0x82 > [<ffffffff81391f6c>] copy_net_ns+0x76/0xe0 > [<ffffffff8107ad60>] create_new_namespaces+0xf0/0x16e > [<ffffffff8107afee>] copy_namespaces+0x65/0x9f > [<ffffffff81056dff>] copy_process+0xb2c/0x12c3 > [<ffffffff810576e1>] do_fork+0x14b/0x2d2 > [<ffffffff8107ac4e>] ? up_read+0xe/0x10 > [<ffffffff81438e73>] ? do_page_fault+0x27a/0x2aa > [<ffffffff8101044b>] sys_clone+0x28/0x2a > [<ffffffff81011fb3>] stub_clone+0x13/0x20 > [<ffffffff81011c72>] ? system_call_fastpath+0x16/0x1b > > And the information of IPV6 in .config is as following. > IPV6 in .config: > CONFIG_IPV6=m > CONFIG_IPV6_PRIVACY=y > CONFIG_IPV6_ROUTER_PREF=y > CONFIG_IPV6_ROUTE_INFO=y > CONFIG_IPV6_OPTIMISTIC_DAD=y > CONFIG_IPV6_MIP6=m > CONFIG_IPV6_SIT=m > # CONFIG_IPV6_SIT_6RD is not set > CONFIG_IPV6_NDISC_NODETYPE=y > CONFIG_IPV6_TUNNEL=m > CONFIG_IPV6_MULTIPLE_TABLES=y > CONFIG_IPV6_SUBTREES=y > CONFIG_IPV6_MROUTE=y > CONFIG_IPV6_PIMSM_V2=y > # CONFIG_IP_VS_IPV6 is not set > CONFIG_NF_CONNTRACK_IPV6=m > CONFIG_IP6_NF_MATCH_IPV6HEADER=m > > I confirmed this patch fixes this problem. > > Signed-off-by: Jin Dongming <jin.dongming@np.css.fujitsu.com> > --- > include/linux/ipv6.h | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h > index ae74ede..5640425 100644 > --- a/include/linux/ipv6.h > +++ b/include/linux/ipv6.h > @@ -208,6 +208,7 @@ enum { > DEVCONF_MC_FORWARDING, > DEVCONF_DISABLE_IPV6, > DEVCONF_ACCEPT_DAD, > + DEVCONF_FORCE_TLLAO, > DEVCONF_MAX > }; > > -- > 1.6.2.2 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch-next] Fix the size overflow of addrconf_sysctl array 2009-10-09 5:44 ` [Patch-next] Fix the size overflow of addrconf_sysctl array David Miller @ 2009-10-09 13:11 ` Cosmin Ratiu 2009-10-13 10:45 ` David Miller 0 siblings, 1 reply; 4+ messages in thread From: Cosmin Ratiu @ 2009-10-09 13:11 UTC (permalink / raw) To: David Miller, opurdila Cc: jin.dongming, kaneshige.kenji, seto.hidetoshi, netdev [-- Attachment #1: Type: Text/Plain, Size: 3799 bytes --] Shouldn't this be changed too then? Or better yet, wouldn't a change that eliminates the need of adding a new option in two separate places be useful? I see the only use for that DEVCONF enum is to dump the settings via netlink. Wouldn't a memcpy suffice? Cosmin. On Friday 09 October 2009 08:44:15 David Miller wrote: > From: Jin Dongming <jin.dongming@np.css.fujitsu.com> > Date: Fri, 09 Oct 2009 11:37:59 +0900 > > Please post networking patches always CC:'d to netdev@vger.kernel.org > so that it gets added to our networking patch tracking system at: > > http://patchwork.ozlabs.org/project/netdev/list/ > > Thank you. > > I've applied your fix, thanks! > > > (This patch fixes bug of commit f7734fdf61ec6bb848e0bafc1fb8bad2c124bb50 > > title "make TLLAO option for NA packets configurable") > > > > When the IPV6 conf is used, the function sysctl_set_parent is called and > > the array addrconf_sysctl is used as a parameter of the function. > > > > The above patch added new conf "force_tllao" into the array > > addrconf_sysctl, but the size of the array was not modified, the static > > allocated size is DEVCONF_MAX + 1 but the real size is DEVCONF_MAX + 2, > > so the problem is that the function sysctl_set_parent accessed wrong > > address. > > > > I got the following information. > > Call Trace: > > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > > [<ffffffff8106085d>] sysctl_set_parent+0x29/0x3e > > [<ffffffff810622d5>] __register_sysctl_paths+0xde/0x272 > > [<ffffffff8110892d>] ? __kmalloc_track_caller+0x16e/0x180 > > [<ffffffffa00cfac3>] ? __addrconf_sysctl_register+0xc5/0x144 [ipv6] > > [<ffffffff8141f2c9>] register_net_sysctl_table+0x48/0x4b > > [<ffffffffa00cfaf5>] __addrconf_sysctl_register+0xf7/0x144 [ipv6] > > [<ffffffffa00cfc16>] addrconf_init_net+0xd4/0x104 [ipv6] > > [<ffffffff8139195f>] setup_net+0x35/0x82 > > [<ffffffff81391f6c>] copy_net_ns+0x76/0xe0 > > [<ffffffff8107ad60>] create_new_namespaces+0xf0/0x16e > > [<ffffffff8107afee>] copy_namespaces+0x65/0x9f > > [<ffffffff81056dff>] copy_process+0xb2c/0x12c3 > > [<ffffffff810576e1>] do_fork+0x14b/0x2d2 > > [<ffffffff8107ac4e>] ? up_read+0xe/0x10 > > [<ffffffff81438e73>] ? do_page_fault+0x27a/0x2aa > > [<ffffffff8101044b>] sys_clone+0x28/0x2a > > [<ffffffff81011fb3>] stub_clone+0x13/0x20 > > [<ffffffff81011c72>] ? system_call_fastpath+0x16/0x1b > > > > And the information of IPV6 in .config is as following. > > IPV6 in .config: > > CONFIG_IPV6=m > > CONFIG_IPV6_PRIVACY=y > > CONFIG_IPV6_ROUTER_PREF=y > > CONFIG_IPV6_ROUTE_INFO=y > > CONFIG_IPV6_OPTIMISTIC_DAD=y > > CONFIG_IPV6_MIP6=m > > CONFIG_IPV6_SIT=m > > # CONFIG_IPV6_SIT_6RD is not set > > CONFIG_IPV6_NDISC_NODETYPE=y > > CONFIG_IPV6_TUNNEL=m > > CONFIG_IPV6_MULTIPLE_TABLES=y > > CONFIG_IPV6_SUBTREES=y > > CONFIG_IPV6_MROUTE=y > > CONFIG_IPV6_PIMSM_V2=y > > # CONFIG_IP_VS_IPV6 is not set > > CONFIG_NF_CONNTRACK_IPV6=m > > CONFIG_IP6_NF_MATCH_IPV6HEADER=m > > > > I confirmed this patch fixes this problem. > > > > Signed-off-by: Jin Dongming <jin.dongming@np.css.fujitsu.com> > > --- > > include/linux/ipv6.h | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h > > index ae74ede..5640425 100644 > > --- a/include/linux/ipv6.h > > +++ b/include/linux/ipv6.h > > @@ -208,6 +208,7 @@ enum { > > DEVCONF_MC_FORWARDING, > > DEVCONF_DISABLE_IPV6, > > DEVCONF_ACCEPT_DAD, > > + DEVCONF_FORCE_TLLAO, > > DEVCONF_MAX > > }; > [-- Attachment #2: 0001-ipv6-fix-devconf-after-adding-force_tllao-option.patch --] [-- Type: text/x-patch, Size: 823 bytes --] From f72949922a102ee9e728a2805287a9bd9a4d2e67 Mon Sep 17 00:00:00 2001 From: Cosmin Ratiu <cratiu@ixiacom.com> Date: Fri, 9 Oct 2009 15:57:09 +0300 Subject: [PATCH] [ipv6]: fix devconf after adding force_tllao option Signed-off-by: Cosmin Ratiu <cratiu@ixiacom.com> --- net/ipv6/addrconf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 1fd0a3d..a065f40 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -3708,6 +3708,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf, #endif array[DEVCONF_DISABLE_IPV6] = cnf->disable_ipv6; array[DEVCONF_ACCEPT_DAD] = cnf->accept_dad; + array[DEVCONF_FORCE_TLLAO] = cnf->force_tllao; } static inline size_t inet6_if_nlmsg_size(void) -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch-next] Fix the size overflow of addrconf_sysctl array 2009-10-09 13:11 ` Cosmin Ratiu @ 2009-10-13 10:45 ` David Miller 2009-10-14 18:14 ` Octavian Purdila 0 siblings, 1 reply; 4+ messages in thread From: David Miller @ 2009-10-13 10:45 UTC (permalink / raw) To: cratiu; +Cc: opurdila, jin.dongming, kaneshige.kenji, seto.hidetoshi, netdev From: Cosmin Ratiu <cratiu@ixiacom.com> Date: Fri, 9 Oct 2009 16:11:14 +0300 > Shouldn't this be changed too then? > > Or better yet, wouldn't a change that eliminates the need of adding a new > option in two separate places be useful? Yes, it's crummy how things work now, indeed. > I see the only use for that DEVCONF enum is to dump the settings via netlink. > Wouldn't a memcpy suffice? It should be. I've applied your patch, thanks! ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch-next] Fix the size overflow of addrconf_sysctl array 2009-10-13 10:45 ` David Miller @ 2009-10-14 18:14 ` Octavian Purdila 0 siblings, 0 replies; 4+ messages in thread From: Octavian Purdila @ 2009-10-14 18:14 UTC (permalink / raw) To: David Miller Cc: cratiu, jin.dongming, kaneshige.kenji, seto.hidetoshi, netdev On Tuesday 13 October 2009 13:45:09 you wrote: > From: Cosmin Ratiu <cratiu@ixiacom.com> > Date: Fri, 9 Oct 2009 16:11:14 +0300 > > > Shouldn't this be changed too then? > > > > Or better yet, wouldn't a change that eliminates the need of adding a new > > option in two separate places be useful? > > Yes, it's crummy how things work now, indeed. > > > I see the only use for that DEVCONF enum is to dump the settings via > > netlink. Wouldn't a memcpy suffice? > > It should be. > I've taken a look at this and it seems the current way of doing it is really required as we need to preserve userspace ABI across different CONFIG_ settings. tavi ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-10-14 18:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4ACEA207.7010208@np.css.fujitsu.com>
2009-10-09 5:44 ` [Patch-next] Fix the size overflow of addrconf_sysctl array David Miller
2009-10-09 13:11 ` Cosmin Ratiu
2009-10-13 10:45 ` David Miller
2009-10-14 18:14 ` Octavian Purdila
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox