netdev - Re: [PATCH net-next 3/3 v9] drivers: net: ethernet: qualcomm: rmnet: Initial implementation

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]

Date:   Mon, 28 Aug 2017 11:40:39 -0600
From:   Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, fengguang.wu@...el.com, dcbw@...hat.com,
        jiri@...nulli.us, stephen@...workplumber.org,
        David.Laight@...lab.com, marcel@...tmann.org, andrew@...n.ch
Subject: Re: [PATCH net-next 3/3 v9] drivers: net: ethernet: qualcomm: rmnet:
 Initial implementation

> 
> In these code paths where you are the writer, you have to rely upon
> the RTNL mutex (or some other mutual exclusion mechanism) to protect
> the update operation.  RCU locking itself does not provide this.
> 
> So you should use something like rcu_dereference_rtnl() or similar.
> 
> So this would be rmnet_force_unassociate_device() and rmnet_dellink()
> 
> RCU is subtle and the writer paths have the be handled differently
> from the reader paths.  Please take some time to study how RCU should
> be applied properly in these situations rather than just slapping a
> patch together overnight.
> 
> Thank you.

Sorry about the mess David.

Can you tell me if the following design is correct -

The shared resource which needs to be protected is 
real_dev->rx_handler_data.

For the writer path, this needs to be protected using rtnl_lock() and 
rcu.
The writer paths are rmnet_newlink(), rmnet_delink() and
rmnet_force_unassociate_device(). These paths are already called with
rtnl_lock() acquired in, so we just need to acquire the rcu_read_lock(). 
To
dereference here, we will need to use rtnl_dereference().

For the reader path, the real_dev->rx_handler_data is called in the TX / 
RX
path. We only need rcu_read_lock() for these scenarios. In these cases,
the rcu_read_lock() is held in __dev_queue_xmit() and
netif_receive_skb_internal(), so readers need to use 
rcu_dereference_rtnl()
to get the relevant information.

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project