diff mbox series

[v2,12/13] qla2xxx: edif: fix edif bsg

Message ID 20211021073208.27582-13-njavali@marvell.com (mailing list archive)
State Superseded
Headers show
Series qla2xxx - misc driver and EDIF bug fixes | expand

Commit Message

Nilesh Javali Oct. 21, 2021, 7:32 a.m. UTC 
From: Quinn Tran <qutran@marvell.com>

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 <qutran@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_edif.c | 49 ++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 26 deletions(-)

Comments

Himanshu Madhani Oct. 21, 2021, 2:46 p.m. UTC | #1 
> On Oct 21, 2021, at 2:32 AM, Nilesh Javali <njavali@marvell.com> wrote:
> 
> From: Quinn Tran <qutran@marvell.com>
> 
> 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 <qutran@marvell.com>
> 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 <himanshu.madhani@oracle.com>

--
Himanshu Madhani	 Oracle Linux Engineering
diff mbox series

Patch

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;
 	}