On 12/20/21 5:39 AM, lixiaokeng wrote:
> |- iscsi_if_destroy_conn |-dev_attr_show
> |-iscsi_conn_teardown
> |-spin_lock_bh |-iscsi_sw_tcp_conn_get_param
>
> |-kfree(conn->persistent_address) |-iscsi_conn_get_param
> |-kfree(conn->local_ipaddr)
> ==>|-read persistent_address
> ==>|-read local_ipaddr
> |-spin_unlock_bh
>
> when iscsi_conn_teardown and iscsi_conn_get_param happen in parallel,
> may trigger UAF issues.
>
> Signed-off-by: Lixiaokeng<lixiaokeng@xxxxxxxxxx>
> Signed-off-by: Linfeilong<linfeilong@xxxxxxxxxx>
> Reported-by: Lu Tixiong<lutianxiong@xxxxxxxxxx>
> ---
> drivers/scsi/libiscsi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 284b939fb1ea..059dae8909ee 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -3100,6 +3100,8 @@ void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)
> {
> struct iscsi_conn *conn = cls_conn->dd_data;
> struct iscsi_session *session = conn->session;
> + char *tmp_persistent_address = conn->persistent_address;
> + char *tmp_local_ipaddr = conn->local_ipaddr;
>
> del_timer_sync(&conn->transport_timer);
>
> @@ -3121,8 +3123,6 @@ void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)
> spin_lock_bh(&session->frwd_lock);
> free_pages((unsigned long) conn->data,
> get_order(ISCSI_DEF_MAX_RECV_SEG_LEN));
> - kfree(conn->persistent_address);
> - kfree(conn->local_ipaddr);
> /* regular RX path uses back_lock */
> spin_lock_bh(&session->back_lock);
> kfifo_in(&session->cmdpool.queue, (void*)&conn->login_task,
> @@ -3134,6 +3134,8 @@ void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)
> mutex_unlock(&session->eh_mutex);
>
> iscsi_destroy_conn(cls_conn);
> + kfree(tmp_persistent_address);
> + kfree(tmp_local_ipaddr);
> }
> EXPORT_SYMBOL_GPL(iscsi_conn_teardown);
>
Reviewed-by: Mike Christie <michael.christie@xxxxxxxxxx>
