lkml.org 
[lkml]   [2015]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH RESEND] bnx2x:Add proper protection from concurrent users in the function bnx2_open
Date
> -----Original Message-----
> From: dept_hsg_linux_nic_dev-bounces@qlclistserver.qlogic.com
> [mailto:dept_hsg_linux_nic_dev-bounces@qlclistserver.qlogic.com] On Behalf
> Of Nicholas Krause
> Sent: Friday, September 25, 2015 3:55 AM
> To: Sony Chacko
> Cc: Dept-GE Linux NIC Dev; linux-kernel; netdev
> Subject: [PATCH RESEND] bnx2x:Add proper protection from concurrent users in
> the function bnx2_open
>
> This fixes bnx2_open to have proper protection from concurrent users as it is
> never properly locked with rtnl_lock/unlock before executing its code that can
> have issues with other threads of execution executing on these data structures
> at the same time. Due to this fix it by making this locking internal to the function
> bnx2_open by unlocking before and after the critical region with the function
> pair rtnl_lock/unlock.
>
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bnx2.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2.c
> b/drivers/net/ethernet/broadcom/bnx2.c
> index 2b66ef3..0c31c49 100644
> --- a/drivers/net/ethernet/broadcom/bnx2.c
> +++ b/drivers/net/ethernet/broadcom/bnx2.c
> @@ -6331,13 +6331,13 @@ bnx2_setup_int_mode(struct bnx2 *bp, int dis_msi)
> return netif_set_real_num_rx_queues(bp->dev, bp->num_rx_rings); }
>
> -/* Called with rtnl_lock */
> static int
> bnx2_open(struct net_device *dev)
> {
> struct bnx2 *bp = netdev_priv(dev);
> int rc;
>
> + rtnl_lock();
> rc = bnx2_request_firmware(bp);
> if (rc < 0)
> goto out;
> @@ -6411,6 +6411,7 @@ open_err:
> bnx2_free_mem(bp);
> bnx2_del_napi(bp);
> bnx2_release_firmware(bp);
> + rtnl_unlock();
> goto out;
> }
>
> --

It's not an appropriate change. Acquiring rtnl_lock() in device's ndo_open() handler will cause deadlock
as OS already take this lock before calling ndo_open().

Also, the subject line is not correct - it was supposed to be bnx2 change, not bnx2x.

Thanks,
Manish



\
 
 \ /
  Last update: 2015-09-25 12:01    [W:0.030 / U:1.448 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site