netdev - Re: [PATCH net] lwtunnel: fix autoload of lwt modules

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: <0e226af4-cb62-9b20-f8a2-191b0aaffb11@brocade.com>
Date:   Tue, 17 Jan 2017 17:26:02 +0000
From:   Robert Shearman <rshearma@...cade.com>
To:     David Ahern <dsa@...ulusnetworks.com>, <netdev@...r.kernel.org>,
        roopa <roopa@...ulusnetworks.com>
Subject: Re: [PATCH net] lwtunnel: fix autoload of lwt modules

On 17/01/17 17:07, David Ahern wrote:
> On 1/17/17 3:04 AM, Robert Shearman wrote:
>> Is it safe to release the rtnl lock here? E.g. neither fib_get_nhs nor fib_create_info take a reference on the device and further up inet_rtm_newroute has a pointer to the struct fib_table without taking a reference.
>
> fib tables can not be deleted so that reference is safe.

Looks like we can free a table on unmerging. Admittedly this is the 
local table so is unlikely to be one that lwtunnels are used on, but 
does it not need to be safe against races anyway?

> You are right about the dev reference in the IPv4 path but the thing is build_state does not need the device reference.
>
> Roopa: do you recall why dev is passed to build_state? Nothing about the encap should require a device reference and none of the current encaps use it.
>
>>
>> Unfortunately, I think more invasive changes are required to solve this. I'm happy to work on solving this.
>
> I have a patch that removes passing dev to build_state. Walking the remainder of the code I do not see any more references that would be affected by dropping rtnl here on the add path. Still looking at the delete path.
>

Ok, I'll continue looking too and let you know if there's anything else 
that pops up.

Having said that, even if we eliminate all the unreferenced objects in 
the current code, what are the chances that we'll be able to keep it 
this way going forward? Perhaps it would be safer to retry the insert 
operation from as close to the start as possible?

Thanks,
Rob