netdev - Re: [PATCH net 0/3] mlx4 fix for shutdown flow

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]

Date:   Fri, 18 Nov 2016 13:47:09 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     tariqt@...lanox.com
Cc:     netdev@...r.kernel.org, eranbe@...lanox.com, saeedm@...lanox.com,
        eugenia@...lanox.com
Subject: Re: [PATCH net 0/3] mlx4 fix for shutdown flow

From: Tariq Toukan <tariqt@...lanox.com>
Date: Thu, 17 Nov 2016 17:40:48 +0200

> This patchset fixes an invalid reference to mdev in mlx4 shutdown flow.
> 
> In patch 1, we make sure netif_device_detach() is called from shutdown flow only,
> since we want to keep it present during a simple configuration change.
> 
> In patches 2 and 3, we add checks that were missing in:
> * dev_get_phys_port_id
> * dev_get_phys_port_name
> We check the presence of the network device before calling the driver's
> callbacks. This already exists for all other ndo's.
> 
> Series generated against net commit:
> e5f6f564fd19 bnxt: add a missing rcu synchronization

I don't like where this is going nor the precedence it is setting.

If you are taking the device into a state where it cannot be safely
accessed by ndo operations, then you _MUST_ do whatever is necessary
to make sure the device is unregistered and cannot be found in the
various global lists and tables of network devices.

This is mandatory.

And this is how we must fix these kinds of problems instead of
peppering device presence test all over the place.  That will be
error prone and in the long term a huge maintainence burdon.

I'm not applying this series, sorry.  You have to fix this properly.