On Sun, Jan 1, 2017 at 9:19 AM, Or Gerlitz <gerlitz.or@xxxxxxxxx> wrote:
> On Sun, Jan 1, 2017 at 8:39 AM, Leon Romanovsky <leonro@xxxxxxxxxxxx> wrote:
>> On Thu, Dec 29, 2016 at 10:55:34PM +0200, Or Gerlitz wrote:
>>> On Wed, Dec 28, 2016 at 2:47 PM, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>>> > From: Feras Daoud <ferasda@xxxxxxxxxxxx>
>>> >
>>> > The ipoib_vlan_add function calls rtnl_unlock after free_netdev,
>>> > rtnl_unlock not only releases the lock, but also calls netdev_run_todo.
>>> > The latter function browses the net_todo_list array and completes the
>>> > unregistration of all its net_device instances. If we call free_netdev
>>> > before rtnl_unlock, then netdev_run_todo call over the freed device causes
>>> > panic.
>>>
>>>
>>> RU claiming that we are crashing 100.0% here? since when? ever?
>>
>> According to our internal bugtracker (see #131352), this bug was
>> literaly forever (4 years old), and it was fixed internaly in
>> MOFED 2.0.
>
> Did you try it out? e.g to see that this crashes 100% as you claim?
>
> I am pretty much sure this is not correct, and I'd like to see Erez commenting
> on that and supporting your claim.
>
It happens in error flow only, when the call for __ipoib_vlan_add
returns with error, in that case we will call free_netdev before
rtnl_unlock.
The fix is similar to what we have in ipoib_vlan_delete function:
"
....
rtnl_unlock();
if (dev) {
free_netdev(dev);
return 0;
}
"
First rtnl_unlock() and then free_netdev
Thanks, Erez
>>> > To fix, move rtnl_unlock call before free_netdev call.
>>> > Fixes: 9baa0b036410 ("IB/ipoib: Add rtnl_link_ops support")
>>> > Cc: Or Gerlitz <ogerlitz@xxxxxxxxxxxx>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
