Re: [PATCH 3.19.y-ckt 091/130] iser-target: Fix REJECT CM event use-after-free OOPs

[Kamal Mostafa <kamal@canonical.com>]

On Thu, 2015-08-27 at 15:11 -0700, Kamal Mostafa wrote:
> 3.19.8-ckt6 -stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------

Per Nicholas' earlier request to the stable ML, I've dropped this patch
from 3.19-stable for now.

 -Kamal


> 
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> commit ce9a9fc20a78ad1e5222fae3a83d105f2d2fb9b9 upstream.
> 
> This patch fixes a bug in iser-target code where the REJECT CM event
> handler code currently performs a isert_put_conn() for the final
> isert_conn->kref put, while iscsi_np process context is still blocked
> in isert_get_login_rx().
> 
> Once isert_get_login_rx() is awoking due to login timeout, iscsi_np
> process context will attempt to invoke iscsi_target_login_sess_out()
> to cleanup iscsi_conn as expected, and calls isert_wait_conn() +
> isert_free_conn() which triggers the use-after-free OOPs.
> 
> To address this bug, move the kref_get_unless_zero() call from
> isert_connected_handler() into isert_connect_request() immediately
> preceeding isert_rdma_accept() to ensure the CM handler cleanup
> paths and isert_free_conn() are always operating with two refs.
> 
> Cc: Sagi Grimberg <sagig@mellanox.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> [ kamal: backport to 3.19-stable: context ]
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> ---
>  drivers/infiniband/ulp/isert/ib_isert.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> index b7350d5..bd28c48 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> @@ -686,6 +686,17 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
>  	ret = isert_rdma_post_recvl(isert_conn);
>  	if (ret)
>  		goto out_conn_dev;
> +	/*
> +	 * Obtain the second reference now before isert_rdma_accept() to
> +	 * ensure that any initiator generated REJECT CM event that occurs
> +	 * asynchronously won't drop the last reference until the error path
> +	 * in iscsi_target_login_sess_out() does it's ->iscsit_free_conn() ->
> +	 * isert_free_conn() -> isert_put_conn() -> kref_put().
> +	 */
> +	if (!kref_get_unless_zero(&isert_conn->conn_kref)) {
> +		isert_warn("conn %p connect_release is running\n", isert_conn);
> +		goto out_conn_dev;
> +	}
>  
>  	ret = isert_rdma_accept(isert_conn);
>  	if (ret)
> @@ -769,11 +780,6 @@ isert_connected_handler(struct rdma_cm_id *cma_id)
>  
>  	isert_info("conn %p\n", isert_conn);
>  
> -	if (!kref_get_unless_zero(&isert_conn->conn_kref)) {
> -		isert_warn("conn %p connect_release is running\n", isert_conn);
> -		return;
> -	}
> -
>  	mutex_lock(&isert_conn->conn_mutex);
>  	if (isert_conn->state != ISER_CONN_FULL_FEATURE)
>  		isert_conn->state = ISER_CONN_UP;