> On Oct 21, 2021, at 2:32 AM, Nilesh Javali <njavali@xxxxxxxxxxx> wrote:
>
> From: Quinn Tran <qutran@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Signed-off-by: Nilesh Javali <njavali@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
--
Himanshu Madhani Oracle Linux Engineering
