diff mbox

[rdma-next,1/4] IB/mlx4: Fix out-of-range array index in destroy qp flow

Message ID 1480252702-8005-2-git-send-email-leon@kernel.org (mailing list archive)
State Accepted
Headers show

Commit Message

Leon Romanovsky Nov. 27, 2016, 1:18 p.m. UTC 
From: Jack Morgenstein <jackm@dev.mellanox.co.il>

For non-special QPs, the port value becomes non-zero only at the
RESET-to-INIT transition. If the QP has not undergone that transition,
its port number value is still zero.

If such a QP is destroyed before being moved out of the RESET state,
subtracting one from the qp port number results in a negative value.
Using that negative value as an index into the qp1_proxy array
results in an out-of-bounds array reference.

Fix this by testing that the QP type is one that uses qp1_proxy before
using the port number. For special QPs of all types, the port number is
specified at QP creation time.

Fixes: 9433c188915c ("IB/mlx4: Invoke UPDATE_QP for proxy QP1 on MAC changes")
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
---
 drivers/infiniband/hw/mlx4/qp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Bart Van Assche Nov. 27, 2016, 4:56 p.m. UTC | #1 
On 11/27/16 05:18, Leon Romanovsky wrote:
> Fixes: 9433c188915c ("IB/mlx4: Invoke UPDATE_QP for proxy QP1 on MAC changes")
> Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> Signed-off-by: Leon Romanovsky <leon@kernel.org>

Hello Leon,

Commit ID 9433c188915c refers to a patch that was merged in kernel 
version v3.15. Shouldn't a "Cc: stable" tag be added to this patch and 
also to the other patches in this series?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Nov. 28, 2016, 8:31 a.m. UTC | #2 
On Sun, Nov 27, 2016 at 04:56:19PM +0000, Bart Van Assche wrote:
> On 11/27/16 05:18, Leon Romanovsky wrote:
> > Fixes: 9433c188915c ("IB/mlx4: Invoke UPDATE_QP for proxy QP1 on MAC changes")
> > Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> > Signed-off-by: Leon Romanovsky <leon@kernel.org>
>
> Hello Leon,
>
> Commit ID 9433c188915c refers to a patch that was merged in kernel
> version v3.15. Shouldn't a "Cc: stable" tag be added to this patch and
> also to the other patches in this series?

I'm extra cautions with stable tags and prefer to finalize my stable
checker in my submissions scripts first, before adding it manually and
I have plans to use it next kernel release.

Thanks

>
> Bart.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bart Van Assche Nov. 28, 2016, 4:29 p.m. UTC | #3 
On 11/28/2016 12:31 AM, Leon Romanovsky wrote:
> I'm extra cautions with stable tags and prefer to finalize my stable
> checker in my submissions scripts first, before adding it manually and
> I have plans to use it next kernel release.

Hello Leon,

Thanks for explaining your workflow. However, the question remains 
whether or not stable tags should be added to the patches in this series?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Dec. 1, 2016, 5:31 a.m. UTC | #4 
On Mon, Nov 28, 2016 at 08:29:55AM -0800, Bart Van Assche wrote:
> On 11/28/2016 12:31 AM, Leon Romanovsky wrote:
> >I'm extra cautions with stable tags and prefer to finalize my stable
> >checker in my submissions scripts first, before adding it manually and
> >I have plans to use it next kernel release.
>
> Hello Leon,
>
> Thanks for explaining your workflow. However, the question remains whether
> or not stable tags should be added to the patches in this series?

It can be added but I don't see any real advantage of it. Do you know
about real users who run RDMA stack from stable trees and don't rely on
distro kernel and/or OFED?

>
> Bart.
Bart Van Assche Dec. 1, 2016, 5:35 a.m. UTC | #5 
On 11/30/16 21:31, Leon Romanovsky wrote:
> On Mon, Nov 28, 2016 at 08:29:55AM -0800, Bart Van Assche wrote:
>> On 11/28/2016 12:31 AM, Leon Romanovsky wrote:
>>> I'm extra cautions with stable tags and prefer to finalize my stable
>>> checker in my submissions scripts first, before adding it manually and
>>> I have plans to use it next kernel release.
>>
>> Thanks for explaining your workflow. However, the question remains whether
>> or not stable tags should be added to the patches in this series?
>
> It can be added but I don't see any real advantage of it. Do you know
> about real users who run RDMA stack from stable trees and don't rely on
> distro kernel and/or OFED?

Hello Leon,

Many users who build an iSER or SRP storage target themselves use a 
recent stable kernel without OFED.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Dec. 1, 2016, 7:02 a.m. UTC | #6 
On Thu, Dec 01, 2016 at 05:35:42AM +0000, Bart Van Assche wrote:
> On 11/30/16 21:31, Leon Romanovsky wrote:
> > On Mon, Nov 28, 2016 at 08:29:55AM -0800, Bart Van Assche wrote:
> >> On 11/28/2016 12:31 AM, Leon Romanovsky wrote:
> >>> I'm extra cautions with stable tags and prefer to finalize my stable
> >>> checker in my submissions scripts first, before adding it manually and
> >>> I have plans to use it next kernel release.
> >>
> >> Thanks for explaining your workflow. However, the question remains whether
> >> or not stable tags should be added to the patches in this series?
> >
> > It can be added but I don't see any real advantage of it. Do you know
> > about real users who run RDMA stack from stable trees and don't rely on
> > distro kernel and/or OFED?
>
> Hello Leon,
>
> Many users who build an iSER or SRP storage target themselves use a
> recent stable kernel without OFED.

Hello Bart,

I'll take into account these users in my future submissions. Meanwhile this
series can be easily picked by various automatic scripts which stable
maintainers are running.

Thanks.

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

Patch

diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 570bc86..6973947 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -1280,7 +1280,8 @@  static int _mlx4_ib_destroy_qp(struct ib_qp *qp)
 	if (is_qp0(dev, mqp))
 		mlx4_CLOSE_PORT(dev->dev, mqp->port);
 
-	if (dev->qp1_proxy[mqp->port - 1] == mqp) {
+	if (mqp->mlx4_ib_qp_type == MLX4_IB_QPT_PROXY_GSI &&
+	    dev->qp1_proxy[mqp->port - 1] == mqp) {
 		mutex_lock(&dev->qp1_proxy_lock[mqp->port - 1]);
 		dev->qp1_proxy[mqp->port - 1] = NULL;
 		mutex_unlock(&dev->qp1_proxy_lock[mqp->port - 1]);