netdev - Re: [PATCH net 1/3] qed: Release CQ resource under lock on failure

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]

Message-ID: <20170125072426.GA4973@yuval-lap>
Date:   Wed, 25 Jan 2017 09:24:27 +0200
From:   Yuval Shaia <yuval.shaia@...cle.com>
To:     Yuval Mintz <Yuval.Mintz@...ium.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        linux-rdma@...r.kernel.org, Ram.Amrani@...ium.com
Subject: Re: [PATCH net 1/3] qed: Release CQ resource under lock on failure

On Tue, Jan 24, 2017 at 11:15:21PM +0200, Yuval Mintz wrote:
> From: Ram Amrani <Ram.Amrani@...ium.com>
> 
> The CQ resource pool is protected by a spin lock. When a CQ creation
> fails it now deallocates under that lock as well.
> 
> Signed-off-by: Ram Amrani <Ram.Amrani@...ium.com>
> Signed-off-by: Yuval Mintz <Yuval.Mintz@...ium.com>
> ---
>  drivers/net/ethernet/qlogic/qed/qed_roce.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c b/drivers/net/ethernet/qlogic/qed/qed_roce.c
> index bd4cad2..7ab6d4e 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
> @@ -948,7 +948,9 @@ static int qed_rdma_create_cq(void *rdma_cxt,
>  
>  err:
>  	/* release allocated icid */
> +	spin_lock_bh(&p_info->lock);
>  	qed_bmap_release_id(p_hwfn, &p_info->cq_map, returned_id);
> +	spin_unlock_bh(&p_info->lock);
>  	DP_NOTICE(p_hwfn, "Create CQ failed, rc = %d\n", rc);

Minor suggestion.
Can you consider embedding the lock and unlock inside qed_bmap_release_id?
There are two places where this is bad idea as driver needs to release two
IDs but still one is in error flow and second is when destroying QP so for
most cases code may look a bit better.

>  
>  	return rc;
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html