netdev - Re: [PATCH] loopback: Force LOOPBACK

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] [day] [month] [year] [list]

Message-Id: <20170621.114427.948405486092143330.davem@davemloft.net>
Date:   Wed, 21 Jun 2017 11:44:27 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     serhe.popovych@...il.com
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] loopback: Force LOOPBACK_IFINDEX for registration

From: Serhey Popovych <serhe.popovych@...il.com>
Date: Wed, 21 Jun 2017 12:35:12 +0300

> 
>> 
>>> From: Serhey Popovych <serhe.popovych@...il.com>
>>> Date: Fri, 16 Jun 2017 15:10:03 +0300
>>>
>>>> Now with commit 9c7dafb (net: Allow to create links with
>>>> given ifindex) support registration of network devices
>>>> with specific ifindex is added.
>>>>
>>>> We can force loopback network device index before call to
>>>> register_netdev() to ensure we always configure it with
>>>> LOOPBACK_IFINDEX.
>>>>
>>>> Kill BUG_ON() since system can continue without network
>>>> namespace failed in loopback init path, unless it is
>>>> init_net namespace where we panic() anyway.
>>>>
>>>> Signed-off-by: Serhey Popovych <serhe.popovych@...il.com>
>>>
>>> Is the BUG_ON() triggering, if so why?
>>>
>>> It looks to me that unless there is a bug, this assignment
>>> is unnecessary.
>>>
>> Okay, get your position, thanks!
>> 
> 
> Sorry for raising again, but my objectives following:
> 
> BUG_ON() might be compiled out when CONFIG_BUG=n.
> That's open possibility in buggy setups to have
> "lo" with ->ifindex other than LOOPBACK_IFINDEX.
> 
> By forcing register_netdevice() via setting
> ifindex = LOOPBACK_IFINDEX we address CONFIG_BUG=n
> case.
> 
> We can leave BUG_ON(), but add ifindex = LOOPBACK_IFINDEX
> to ensure register_netdevice() operates properly.
> 
> What do you think?

I don't understand what you are trying to achieve.

BUG_ON() is an assertion.

It checks that an invariant holds.

Whether the check is enabled or not has no bearing on the
invariant.

There is no reason to make the assignment if it is supposed
to be set properly already, period.  The assertion states
that it is supposed to be set properly when we get to this
code.