From: David Ahern <dsahern@gmail•com>
To: Sabrina Dubroca <sd@queasysnail•net>
Cc: netdev@vger•kernel.org, Jiri Pirko <jiri@resnulli•us>,
Felix Jia <felix.jia@alliedtelesis•co.nz>
Subject: Re: [PATCH net 1/3] net/ipv6: fix addrconf_sysctl_addr_gen_mode
Date: Fri, 6 Jul 2018 09:28:48 -0600 [thread overview]
Message-ID: <99f4b357-741a-e6e4-1ced-bf70aa9a0262@gmail.com> (raw)
In-Reply-To: <20180706150214.GA25283@bistromath.localdomain>
On 7/6/18 9:02 AM, Sabrina Dubroca wrote:
> 2018-07-06, 08:42:01 -0600, David Ahern wrote:
>> On 7/6/18 7:49 AM, Sabrina Dubroca wrote:
>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>> index 91580c62bb86..e9ba53d2a147 100644
>>> --- a/net/ipv6/addrconf.c
>>> +++ b/net/ipv6/addrconf.c
>>> @@ -5892,32 +5892,31 @@ static int addrconf_sysctl_addr_gen_mode(struct ctl_table *ctl, int write,
>>> loff_t *ppos)
>>> {
>>> int ret = 0;
>>> - int new_val;
>>> + u32 new_val;
>>> struct inet6_dev *idev = (struct inet6_dev *)ctl->extra1;
>>> struct net *net = (struct net *)ctl->extra2;
>>> + struct ctl_table tmp = {
>>> + .data = &new_val,
>>> + .maxlen = sizeof(new_val),
>>> + .mode = ctl->mode,
>>> + };
>>>
>>> if (!rtnl_trylock())
>>> return restart_syscall();
>>>
>>> - ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
>>> + new_val = *((u32 *)ctl->data);
>>>
>>> - if (write) {
>>> - new_val = *((int *)ctl->data);
>>> + ret = proc_douintvec(&tmp, write, buffer, lenp, ppos);
>>> + if (ret != 0)
>>> + goto out;
>>>
>>> + if (write) {
>>> if (check_addr_gen_mode(new_val) < 0) {
>>> ret = -EINVAL;
>>> goto out;
>>> }
>>>
>>> - /* request for default */
>>> - if (&net->ipv6.devconf_dflt->addr_gen_mode == ctl->data) {
>>> - ipv6_devconf_dflt.addr_gen_mode = new_val;
>>
>> updating the template is wrong, but you still need to update the
>> namespace's default value for new devices.
>
> That's already handled by storing new_val into ctl->data at the end of
> the 'write' block.
ok. missed that. It's part of your change below.
>
> BTW, I'd like to make ipv6_devconf and ipv6_devconf_dflt read-only, so
> that people aren't tempted to update the template, but I'm thinking of
> doing that in net-next rather than net.
>
>> also, if you are fixing this it would be good to handle the change to
>> 'all' as well and update all existing devices.
>
> I thought about it, and wasn't sure if that change of behavior was
> acceptable, especially for stable (I think the current patch should go
> into stable). OTOH, it's quite clearly what "all" should do.
IMHO it's a bug that changing 'all' does not actually change all
existing devices nor is it ever used.
Looking at other addr_gen_mode sites, addrconf_sysctl_stable_secret is
messed up as well. It propagates a change to 'default' to all existing
devices.
next prev parent reply other threads:[~2018-07-06 15:28 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-06 13:49 [PATCH net 0/3] net/ipv6: addr_gen_mode fixes Sabrina Dubroca
2018-07-06 13:49 ` [PATCH net 1/3] net/ipv6: fix addrconf_sysctl_addr_gen_mode Sabrina Dubroca
2018-07-06 14:42 ` David Ahern
2018-07-06 15:02 ` Sabrina Dubroca
2018-07-06 15:28 ` David Ahern [this message]
2018-07-06 15:58 ` Sabrina Dubroca
2018-07-06 23:28 ` David Ahern
2018-07-06 13:49 ` [PATCH net 2/3] net/ipv6: don't reinitialize ndev->cnf.addr_gen_mode on new inet6_dev Sabrina Dubroca
2018-07-06 14:42 ` David Ahern
2018-07-06 13:49 ` [PATCH net 3/3] net/ipv6: reserve room for IFLA_INET6_ADDR_GEN_MODE Sabrina Dubroca
2018-07-06 14:45 ` David Ahern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=99f4b357-741a-e6e4-1ced-bf70aa9a0262@gmail.com \
--to=dsahern@gmail$(echo .)com \
--cc=felix.jia@alliedtelesis$(echo .)co.nz \
--cc=jiri@resnulli$(echo .)us \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sd@queasysnail$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox