netdev - Re: [PATCH net] ipvlan: call dev_change

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180621011843.GX8958@leo.usersys.redhat.com>
Date:   Thu, 21 Jun 2018 09:18:43 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Stefano Brivio <sbrivio@...hat.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Mahesh Bandewar <maheshb@...gle.com>
Subject: Re: [PATCH net] ipvlan: call dev_change_flags when reset ipvlan mode

On Wed, Jun 20, 2018 at 10:45:39AM -0700, Cong Wang wrote:
> On Tue, Jun 19, 2018 at 10:31 PM, David Miller <davem@...emloft.net> wrote:
> > From: Hangbin Liu <liuhangbin@...il.com>
> > Date: Wed, 20 Jun 2018 11:22:54 +0800
> >
> >> The only case dev_change_flags() return an err is when we change IFF_UP flag.
> >> Since we only set/reset IFF_NOARP, do you think we still need to check the
> >> return value?
> >
> > It is bad to try and take shortcuts on error handling using assumptions
> > like that.
> >
> > If dev_change_flags() is adjusted to return error codes in more
> > situations, nobody is going to remember to undo your "optimziation"
> > here.
> >
> > Please check for errors, thank you.
> 
> Yeah. Also since the notifier is triggered in this case:
> 
>         if (dev->flags & IFF_UP &&
>             (changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE))) {
>                 struct netdev_notifier_change_info change_info = {
>                         .info = {
>                                 .dev = dev,
>                         },
>                         .flags_changed = changes,
>                 };
> 
>                 call_netdevice_notifiers_info(NETDEV_CHANGE, &change_info.info);
>         }
> 
> the return value of call_netdevice_notifiers_info() isn't captured
> either, but it should be.

Thanks for the explanation. I will fix it.

Regards
Hangbin