Thread (27 messages) 27 messages, 2 authors, 2021-10-21

Re: [PATCH v2 12/13] qla2xxx: edif: fix edif bsg

From: Himanshu Madhani <hidden>
Date: 2021-10-21 14:46:35

quoted hunk ↗ jump to hunk
On Oct 21, 2021, at 2:32 AM, Nilesh Javali [off-list ref] wrote:

From: Quinn Tran <redacted>

Various EDIF bsg's did not properly fill out the reply_payload_rcv_len
field. This cause app to parse empty data in the return payload.

Fixes: 7ebb336e45ef ("scsi: qla2xxx: edif: Add start + stop bsgs")
Signed-off-by: Quinn Tran <redacted>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
drivers/scsi/qla2xxx/qla_edif.c | 49 ++++++++++++++++-----------------
1 file changed, 23 insertions(+), 26 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_edif.c b/drivers/scsi/qla2xxx/qla_edif.c
index 440a3caa28f9..68ae7ab43d0c 100644
--- a/drivers/scsi/qla2xxx/qla_edif.c
+++ b/drivers/scsi/qla2xxx/qla_edif.c
@@ -546,14 +546,14 @@ qla_edif_app_start(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
	appreply.edif_enode_active = vha->pur_cinfo.enode_flags;
	appreply.edif_edb_active = vha->e_dbell.db_flags;

-	bsg_job->reply_len = sizeof(struct fc_bsg_reply) +
-	    sizeof(struct app_start_reply);
+	bsg_job->reply_len = sizeof(struct fc_bsg_reply);

	SET_DID_STATUS(bsg_reply->result, DID_OK);

-	sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
-	    bsg_job->reply_payload.sg_cnt, &appreply,
-	    sizeof(struct app_start_reply));
+	bsg_reply->reply_payload_rcv_len = sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
+							       bsg_job->reply_payload.sg_cnt,
+							       &appreply,
+							       sizeof(struct app_start_reply));

	ql_dbg(ql_dbg_edif, vha, 0x911d,
	    "%s app start completed with 0x%x\n",
@@ -750,9 +750,10 @@ qla_edif_app_authok(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
errstate_exit:
	bsg_job->reply_len = sizeof(struct fc_bsg_reply);
-	sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
-	    bsg_job->reply_payload.sg_cnt, &appplogireply,
-	    sizeof(struct app_plogi_reply));
+	bsg_reply->reply_payload_rcv_len = sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
+							       bsg_job->reply_payload.sg_cnt,
+							       &appplogireply,
+							       sizeof(struct app_plogi_reply));

	return rval;
}
@@ -835,7 +836,7 @@ static int
qla_edif_app_getfcinfo(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
{
	int32_t			rval = 0;
-	int32_t			num_cnt;
+	int32_t			pcnt = 0;
	struct fc_bsg_reply	*bsg_reply = bsg_job->reply;
	struct app_pinfo_req	app_req;
	struct app_pinfo_reply	*app_reply;
@@ -847,16 +848,14 @@ qla_edif_app_getfcinfo(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
	    bsg_job->request_payload.sg_cnt, &app_req,
	    sizeof(struct app_pinfo_req));

-	num_cnt = app_req.num_ports;	/* num of ports alloc'd by app */
-
	app_reply = kzalloc((sizeof(struct app_pinfo_reply) +
-	    sizeof(struct app_pinfo) * num_cnt), GFP_KERNEL);
+	    sizeof(struct app_pinfo) * app_req.num_ports), GFP_KERNEL);
+
	if (!app_reply) {
		SET_DID_STATUS(bsg_reply->result, DID_ERROR);
		rval = -1;
	} else {
		struct fc_port	*fcport = NULL, *tf;
-		uint32_t	pcnt = 0;

		list_for_each_entry_safe(fcport, tf, &vha->vp_fcports, list) {
			if (!(fcport->flags & FCF_FCSP_DEVICE))
@@ -925,9 +924,11 @@ qla_edif_app_getfcinfo(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
		SET_DID_STATUS(bsg_reply->result, DID_OK);
	}

-	sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
-	    bsg_job->reply_payload.sg_cnt, app_reply,
-	    sizeof(struct app_pinfo_reply) + sizeof(struct app_pinfo) * num_cnt);
+	bsg_job->reply_len = sizeof(struct fc_bsg_reply);
+	bsg_reply->reply_payload_rcv_len = sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
+							       bsg_job->reply_payload.sg_cnt,
+							       app_reply,
+							       sizeof(struct app_pinfo_reply) + sizeof(struct app_pinfo) * pcnt);

	kfree(app_reply);
@@ -944,10 +945,11 @@ qla_edif_app_getstats(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
{
	int32_t			rval = 0;
	struct fc_bsg_reply	*bsg_reply = bsg_job->reply;
-	uint32_t ret_size, size;
+	uint32_t size;

	struct app_sinfo_req	app_req;
	struct app_stats_reply	*app_reply;
+	uint32_t pcnt = 0;

	sg_copy_to_buffer(bsg_job->request_payload.sg_list,
	    bsg_job->request_payload.sg_cnt, &app_req,
@@ -963,18 +965,12 @@ qla_edif_app_getstats(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
	size = sizeof(struct app_stats_reply) +
	    (sizeof(struct app_sinfo) * app_req.num_ports);

-	if (size > bsg_job->reply_payload.payload_len)
-		ret_size = bsg_job->reply_payload.payload_len;
-	else
-		ret_size = size;
-
	app_reply = kzalloc(size, GFP_KERNEL);
	if (!app_reply) {
		SET_DID_STATUS(bsg_reply->result, DID_ERROR);
		rval = -1;
	} else {
		struct fc_port	*fcport = NULL, *tf;
-		uint32_t	pcnt = 0;

		list_for_each_entry_safe(fcport, tf, &vha->vp_fcports, list) {
			if (fcport->edif.enable) {
@@ -998,9 +994,11 @@ qla_edif_app_getstats(scsi_qla_host_t *vha, struct bsg_job *bsg_job)
		SET_DID_STATUS(bsg_reply->result, DID_OK);
	}

+	bsg_job->reply_len = sizeof(struct fc_bsg_reply);
	bsg_reply->reply_payload_rcv_len =
	    sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
-	       bsg_job->reply_payload.sg_cnt, app_reply, ret_size);
+	       bsg_job->reply_payload.sg_cnt, app_reply,
+	       sizeof(struct app_stats_reply) + (sizeof(struct app_sinfo) * pcnt));

	kfree(app_reply);
@@ -1074,8 +1072,7 @@ qla_edif_app_mgmt(struct bsg_job *bsg_job)
		    __func__,
		    bsg_request->rqst_data.h_vendor.vendor_cmd[1]);
		rval = EXT_STATUS_INVALID_PARAM;
-		bsg_job->reply_len = sizeof(struct fc_bsg_reply);
-		SET_DID_STATUS(bsg_reply->result, DID_ERROR);
+		done = false;
		break;
	}

-- 
2.19.0.rc0
Reviewed-by: Himanshu Madhani <redacted>

--
Himanshu Madhani	 Oracle Linux Engineering
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help