[PATCH for-next] RDMA/rxe: Fix double free in rxe_qp.c

[PATCH for-next] RDMA/rxe: Fix double free in rxe_qp.c

[Bob Pearson]

A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr()
at line 715 in rxe_qp.c. This patch corrects that behavior.

A newer patch from Guoqing Jiang recommends replacing all spin_lock
calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since
apparently the blktests test suite can call the kernel verbs APIs
while in hard interrupt state. This patch needs to be applied first
and Guoqing's patch modified to accommodate this small change.

Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/
Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index c5451a4488ca..245dd36638c7 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -712,8 +712,9 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask)
 	if (qp->attr.sq_draining) {
 		spin_unlock_bh(&qp->state_lock);
 		cond_resched();
+	} else {
+		spin_unlock_bh(&qp->state_lock);
 	}
-	spin_unlock_bh(&qp->state_lock);
 
 	return 0;
 }
-- 
2.37.2

Re: [PATCH for-next] RDMA/rxe: Fix double free in rxe_qp.c

[Guoqing Jiang]

Hello,

On 5/16/23 04:10, Bob Pearson wrote:
> A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr()
> at line 715 in rxe_qp.c. This patch corrects that behavior.
>
> A newer patch from Guoqing Jiang recommends replacing all spin_lock
> calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since
> apparently the blktests test suite can call the kernel verbs APIs
> while in hard interrupt state. This patch needs to be applied first
> and Guoqing's patch modified to accommodate this small change.

If you don't mind, I will send a patch set with your patch as first one, 
then
refresh mine. Which means we don't need to keep the second paragraph
in commit message, what do you think?

> Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>   drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index c5451a4488ca..245dd36638c7 100644
> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
> @@ -712,8 +712,9 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask)
>   	if (qp->attr.sq_draining) {
>   		spin_unlock_bh(&qp->state_lock);
>   		cond_resched();
> +	} else {
> +		spin_unlock_bh(&qp->state_lock);
>   	}
> -	spin_unlock_bh(&qp->state_lock);
>   
>   	return 0;
>   }

Looks good, Acked-by: Guoqing Jiang <guoqing.jiang@linux.dev>

Thanks,
Guoqing

Re: [PATCH for-next] RDMA/rxe: Fix double free in rxe_qp.c

[Bob Pearson]

On 5/15/23 21:10, Guoqing Jiang wrote:
> Hello,
> 
> On 5/16/23 04:10, Bob Pearson wrote:
>> A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr()
>> at line 715 in rxe_qp.c. This patch corrects that behavior.
>>
>> A newer patch from Guoqing Jiang recommends replacing all spin_lock
>> calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since
>> apparently the blktests test suite can call the kernel verbs APIs
>> while in hard interrupt state. This patch needs to be applied first
>> and Guoqing's patch modified to accommodate this small change.
> 
> If you don't mind, I will send a patch set with your patch as first one, then
> refresh mine. Which means we don't need to keep the second paragraph
> in commit message, what do you think?
> 
>> Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock")
>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>> Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/
>> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
>> ---
>>   drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
>> index c5451a4488ca..245dd36638c7 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
>> @@ -712,8 +712,9 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask)
>>       if (qp->attr.sq_draining) {
>>           spin_unlock_bh(&qp->state_lock);
>>           cond_resched();
>> +    } else {
>> +        spin_unlock_bh(&qp->state_lock);
>>       }
>> -    spin_unlock_bh(&qp->state_lock);
>>         return 0;
>>   }
> 
> Looks good, Acked-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> 
> Thanks,
> Guoqing

Guoqing,

I don't care how we do it. Perhaps we should leave it up to the maintainers.
Just needs to be done.

Bob

Re: [PATCH for-next] RDMA/rxe: Fix double free in rxe_qp.c

[Jason Gunthorpe]

On Mon, May 15, 2023 at 03:10:57PM -0500, Bob Pearson wrote:
> A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr()
> at line 715 in rxe_qp.c. This patch corrects that behavior.
> 
> A newer patch from Guoqing Jiang recommends replacing all spin_lock
> calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since
> apparently the blktests test suite can call the kernel verbs APIs
> while in hard interrupt state. This patch needs to be applied first
> and Guoqing's patch modified to accommodate this small change.
> 
> Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to for-rc, thanks

Jason

Re: [PATCH for-next] RDMA/rxe: Fix double free in rxe_qp.c

[Bob Pearson]

On 5/16/23 19:11, Jason Gunthorpe wrote:
> On Mon, May 15, 2023 at 03:10:57PM -0500, Bob Pearson wrote:
>> A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr()
>> at line 715 in rxe_qp.c. This patch corrects that behavior.
>>
>> A newer patch from Guoqing Jiang recommends replacing all spin_lock
>> calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since
>> apparently the blktests test suite can call the kernel verbs APIs
>> while in hard interrupt state. This patch needs to be applied first
>> and Guoqing's patch modified to accommodate this small change.
>>
>> Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock")
>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>> Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/
>> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
>> ---
>>  drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Applied to for-rc, thanks
> 
> Jason

Thanks