Re: [PATCH] IB/rxe: double free on error

[Leon Romanovsky <leon@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

On Wed, Mar 08, 2017 at 08:21:52AM +0300, Dan Carpenter wrote:
> "goto err;" has it's own kfree_skb() call so it's a double free.  We
> only need to free on the "goto exit;" path.
>
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static analysis.  Not tested.  Please review carefully.
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index dbfde0dc6ff7..9f95f50b2909 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -729,11 +729,11 @@ int rxe_requester(void *arg)
>  	ret = rxe_xmit_packet(to_rdev(qp->ibqp.device), qp, &pkt, skb);

This rxe_xmit_packet() looks a little bit awkward. It calls to
kfree_skb and returns ret = 0 after drop decision. It doesn't
free on error (ret != 0), but this rxe_requester does.

However in case of not error, the skb won't be released and goto
next_cqe will be called, which has a lot of exit paths without freeing
skb.

Moni, Yonatan
Is it done on purpose?


>  	if (ret) {
>  		qp->need_req_skb = 1;
> -		kfree_skb(skb);
>
>  		rollback_state(wqe, qp, &rollback_wqe, rollback_psn);
>
>  		if (ret == -EAGAIN) {
> +			kfree_skb(skb);
>  			rxe_run_task(&qp->req.task, 1);
>  			goto exit;
>  		}
> --
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]