netdev - Re: [PATCH net 5/6] rtnetlink: check DO_SETLINK_NOTIFY correctly in do

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: <0e37130c-bbf3-c75e-5cee-c3a08206873f@redhat.com>
Date:   Thu, 26 Oct 2017 08:52:48 +0200
From:   Vlad Yasevich <vyasevic@...hat.com>
To:     nicolas.dichtel@...nd.com, David Ahern <dsahern@...il.com>,
        Xin Long <lucien.xin@...il.com>,
        network dev <netdev@...r.kernel.org>
Cc:     davem@...emloft.net, hannes@...essinduktion.org
Subject: Re: [PATCH net 5/6] rtnetlink: check DO_SETLINK_NOTIFY correctly in
 do_setlink

On 10/16/2017 02:20 PM, Nicolas Dichtel wrote:
> Le 16/10/2017 à 03:17, David Ahern a écrit :
>> [ cc'ed Nicolas ]
>>
>> On 10/15/17 4:13 AM, Xin Long wrote:
>>> The check 'status & DO_SETLINK_NOTIFY' in do_setlink doesn't really
>>> work after status & DO_SETLINK_MODIFIED, as:
>>>
>>>   DO_SETLINK_MODIFIED 0x1
>>>   DO_SETLINK_NOTIFY 0x3
>>>
>>> Considering that notifications are suppposed to be sent only when
>>> status have the flag DO_SETLINK_NOTIFY, the right check would be:
>>>
>>>   (status & DO_SETLINK_NOTIFY) == DO_SETLINK_NOTIFY
>>>
>>> This would avoid lots of duplicated notifications when setting some
>>> properties of a link.
>>>
>>> Fixes: ba9989069f4e ("rtnl/do_setlink(): notify when a netdev is modified")
>>> Signed-off-by: Xin Long <lucien.xin@...il.com>
> Good catch, thank you.
> 
> Acked-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>
> 

So I found this the first timer around when looking at this code, but was told that
notification are expected anytime we modified any setting thus the code was simply
checking for MODIFIED bit.  Has that thinking changed?

-vlad