* [Patch-next] Fix the size overflow of addrconf_sysctl array
@ 2009-10-09 2:37 Jin Dongming
2009-10-09 5:44 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Jin Dongming @ 2009-10-09 2:37 UTC (permalink / raw)
To: LKLM
Cc: Cosmin Ratiu, Octavian Purdila, David S. Miller, Kenji Kaneshige,
Hidetoshi Seto
(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 related [flat|nested] 5+ messages in thread* Re: [Patch-next] Fix the size overflow of addrconf_sysctl array
2009-10-09 2:37 [Patch-next] Fix the size overflow of addrconf_sysctl array Jin Dongming
@ 2009-10-09 5:44 ` David Miller
2009-10-09 13:11 ` Cosmin Ratiu
0 siblings, 1 reply; 5+ 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] 5+ messages in thread* Re: [Patch-next] Fix the size overflow of addrconf_sysctl array
2009-10-09 5:44 ` David Miller
@ 2009-10-09 13:11 ` Cosmin Ratiu
2009-10-13 10:45 ` David Miller
0 siblings, 1 reply; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2009-10-14 18:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-09 2:37 [Patch-next] Fix the size overflow of addrconf_sysctl array Jin Dongming
2009-10-09 5:44 ` 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 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.