netdev - Re: [PATCH net] bridge: mdb: fix delmdb state in the notification

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: <55B80589.4070500@cumulusnetworks.com>
Date:	Wed, 29 Jul 2015 00:43:21 +0200
From:	Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:	Cong Wang <cwang@...pensource.com>,
	Nikolay Aleksandrov <razor@...ckwall.org>
Cc:	netdev <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	"bridge@...ts.linux-foundation.org" 
	<bridge@...ts.linux-foundation.org>,
	Stephen Hemminger <stephen@...workplumber.org>,
	Cong Wang <amwang@...hat.com>
Subject: Re: [PATCH net] bridge: mdb: fix delmdb state in the notification

On 07/29/2015 12:38 AM, Cong Wang wrote:
> On Tue, Jul 28, 2015 at 4:10 AM, Nikolay Aleksandrov
> <razor@...ckwall.org> wrote:
>> From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>>
>> Since mdb states were introduced when deleting an entry the state was
>> left as it was set in the delete request from the user which leads to
>> the following output when doing a monitor (for example):
>> $ bridge mdb add dev br0 port eth3 grp 239.0.0.1 permanent
>> (monitor) dev br0 port eth3 grp 239.0.0.1 permanent
>> $ bridge mdb del dev br0 port eth3 grp 239.0.0.1 permanent
>> (monitor) dev br0 port eth3 grp 239.0.0.1 temp
>> ^^^
>> Note the "temp" state in the delete notification which is wrong since
>> the entry was permanent, the state in a delete is always reported as
>> "temp" regardless of the real state of the entry.
>>
> 
> Hmm?
> 
> I think it is iproute2 who forgets to set entry->state when deleting it?
> 
>                 } else if (strcmp(*argv, "permanent") == 0) {
>                         if (cmd == RTM_NEWMDB)
>                                 entry.state |= MDB_PERMANENT;
> 
> Kernel simply returns what you pass to it.
> 
> Please fix iproute2.
> 

Hi Cong,
Please read the full commit log, I've explained that the state is not honored in the kernel
so it doesn't matter if iproute2 sets the correct state that you give on the command
line, that is if I give it "temp" and the entry is permanent - it will still get
deleted and the notification will have the wrong state as "temp" because I've set
it, while this way it'll at least return the correct state of the entry being deleted.
Again I'm saying that I chose this solution over a check for the entry state because
it may break some user-space tools that rely on the behaviour that the state is
not checked in the kernel.

Cheers,
 Nik
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html