Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1232685 > unrolled thread

RE: [PATCH RESEND] bnx2x:Add proper protection from concurrent users in the function bnx2_open

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH RESEND] bnx2x:Add proper protection from concurrent  users in the function bnx2_open Manish Chopra <manish.chopra@qlogic.com> - 2015-09-25 11:40 +0200

#1232685 — RE: [PATCH RESEND] bnx2x:Add proper protection from concurrent users in the function bnx2_open

> -----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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [standalone]

Back to top | Article view | linux.kernel

csiph-web