Commit 22698483 authored by Varun Prakash's avatar Varun Prakash Committed by Martin K. Petersen
Browse files

scsi: libcxgbi: simplify task->hdr allocation for mgmt cmds 



In case of mgmt cmds, task->hdr is dereferenced after transmitting the
pdu in iscsi_tcp_task_xmit(). To handle this case current code
increments the Tx skb reference count and frees the skb in
cxgbi_cleanup_task(). In some error cases this results in skb leak.

To fix this in case of mgmt cmds, allocate a separate buffer for iSCSI
hdr and free this buffer in cxgbi_cleanup_task().

Signed-off-by: default avatarVarun Prakash <varun@chelsio.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 9b3a081f

drivers/scsi/cxgbi/libcxgbi.c

+28 −15
Original line number Diff line number Diff line
@@ -1887,16 +1887,13 @@ int cxgbi_conn_alloc_pdu(struct iscsi_task *task, u8 opcode) 
	struct iscsi_tcp_task *tcp_task = task->dd_data;

	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);

	struct scsi_cmnd *sc = task->sc;

	struct cxgbi_sock *csk = cconn->cep->csk;

	struct net_device *ndev = cdev->ports[csk->port_id];

	int headroom = SKB_TX_ISCSI_PDU_HEADER_MAX;



	tcp_task->dd_data = tdata;

	task->hdr = NULL;



	if (tdata->skb) {

		kfree_skb(tdata->skb);

		tdata->skb = NULL;

	}



	if (SKB_MAX_HEAD(cdev->skb_tx_rsvd) > (512 * MAX_SKB_FRAGS) &&

	    (opcode == ISCSI_OP_SCSI_DATA_OUT ||

	     (opcode == ISCSI_OP_SCSI_CMD &&
@@ -1908,15 +1905,23 @@ int cxgbi_conn_alloc_pdu(struct iscsi_task *task, u8 opcode) 


	tdata->skb = alloc_skb(cdev->skb_tx_rsvd + headroom, GFP_ATOMIC);

	if (!tdata->skb) {

		struct cxgbi_sock *csk = cconn->cep->csk;

		struct net_device *ndev = cdev->ports[csk->port_id];

		ndev->stats.tx_dropped++;

		return -ENOMEM;

	}



	skb_get(tdata->skb);

	skb_reserve(tdata->skb, cdev->skb_tx_rsvd);



	if (task->sc) {

		task->hdr = (struct iscsi_hdr *)tdata->skb->data;

	} else {

		task->hdr = kzalloc(SKB_TX_ISCSI_PDU_HEADER_MAX, GFP_KERNEL);

		if (!task->hdr) {

			__kfree_skb(tdata->skb);

			tdata->skb = NULL;

			ndev->stats.tx_dropped++;

			return -ENOMEM;

		}

	}

	task->hdr_max = SKB_TX_ISCSI_PDU_HEADER_MAX; /* BHS + AHS */



	/* data_out uses scsi_cmd's itt */
@@ -2060,9 +2065,9 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task) 
	unsigned int datalen;

	int err;



	if (!skb || cxgbi_skcb_test_flag(skb, SKCBF_TX_DONE)) {

	if (!skb) {

		log_debug(1 << CXGBI_DBG_ISCSI | 1 << CXGBI_DBG_PDU_TX,

			"task 0x%p, skb 0x%p\n", task, skb);

			"task 0x%p\n", task);

		return 0;

	}
@@ -2074,6 +2079,7 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task) 
		return -EPIPE;

	}



	tdata->skb = NULL;

	datalen = skb->data_len;



	/* write ppod first if using ofldq to write ppod */
@@ -2087,6 +2093,9 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task) 
			/* continue. Let fl get the data */

	}



	if (!task->sc)

		memcpy(skb->data, task->hdr, SKB_TX_ISCSI_PDU_HEADER_MAX);



	err = cxgbi_sock_send_pdus(cconn->cep->csk, skb);

	if (err > 0) {

		int pdulen = err;
@@ -2102,7 +2111,6 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task) 
			pdulen += ISCSI_DIGEST_SIZE;



		task->conn->txdata_octets += pdulen;

		cxgbi_skcb_set_flag(skb, SKCBF_TX_DONE);

		return 0;

	}
@@ -2111,6 +2119,7 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task) 
			"task 0x%p, skb 0x%p, len %u/%u, %d EAGAIN.\n",

			task, skb, skb->len, skb->data_len, err);

		/* reset skb to send when we are called again */

		tdata->skb = skb;

		return err;

	}
@@ -2118,8 +2127,7 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task) 
		"itt 0x%x, skb 0x%p, len %u/%u, xmit err %d.\n",

		task->itt, skb, skb->len, skb->data_len, err);



	__kfree_skb(tdata->skb);

	tdata->skb = NULL;

	__kfree_skb(skb);



	iscsi_conn_printk(KERN_ERR, task->conn, "xmit err %d.\n", err);

	iscsi_conn_failure(task->conn, ISCSI_ERR_XMIT_FAILED);
@@ -2144,9 +2152,14 @@ void cxgbi_cleanup_task(struct iscsi_task *task) 
		task, tdata->skb, task->hdr_itt);



	tcp_task->dd_data = NULL;



	if (!task->sc)

		kfree(task->hdr);

	task->hdr = NULL;



	/*  never reached the xmit task callout */

	if (tdata->skb) {

		kfree_skb(tdata->skb);

		__kfree_skb(tdata->skb);

		tdata->skb = NULL;

	}

drivers/scsi/cxgbi/libcxgbi.h

+0 −1
Original line number Diff line number Diff line
@@ -205,7 +205,6 @@ enum cxgbi_skcb_flags { 
	SKCBF_TX_NEED_HDR,	/* packet needs a header */

	SKCBF_TX_MEM_WRITE,     /* memory write */

	SKCBF_TX_FLAG_COMPL,    /* wr completion flag */

	SKCBF_TX_DONE,		/* skb tx done */

	SKCBF_RX_COALESCED,	/* received whole pdu */

	SKCBF_RX_HDR,		/* received pdu header */

	SKCBF_RX_DATA,		/* received pdu payload */

CRA Git | Maintained and supported by SUSTech CRA and CCSE