libiscsi: Use scsi helper to set information descriptor

Hello Sagi Grimberg,

The patch a73c2a2f9123: "libiscsi: Use scsi helper to set information descriptor" from Jul 15, 2015, leads to the following static checker warning:

drivers/scsi/libiscsi.c:858 iscsi_scsi_cmd_rsp()
error: XXX uninitialized symbol 'sector'.

drivers/scsi/libiscsi.c
850 ascq = session->tt->check_protection(task, &sector);

If "ascq" is 0x1 then there sector might not be initialized. The documentation is not clear on how that works. Har dee har har. The oldest jokes are still the best... :P

   851                  if (ascq) {
   852                          sc->result = DRIVER_SENSE << 24 |
   853                                       SAM_STAT_CHECK_CONDITION;
   854                          scsi_build_sense_buffer(1, sc->sense_buffer,
   855                                                  ILLEGAL_REQUEST, 0x10, ascq);
   856                          scsi_set_sense_information(sc->sense_buffer,
   857                                                     SCSI_SENSE_BUFFERSIZE,
   858                                                     sector);
   859                          goto out;
   860                  }

regards,
dan carpenter

Hey Dan,

iscsi transports that implement this callout are expected to set the sector which is passed by reference.

would it make the checker happy if we set sector to 0 before calling check_protection (although it's not needed by no means)?

There is only iscsi_iser_check_protection() I think. It behaves how I described in my original email. If ib_check_mr_status() fails it returns without setting sector.

It looks for if there is any possible way that it could be uninitialized so that would solve the problem.

regards,
dan carpenter

You saw the bug in the end right?

1112 u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
1113 enum iser_data_dir cmd_dir, sector_t *sector)

                                                                           ^^^^^^

The caller assumes sector is initialized on error.

  1114  {
  1115          struct iser_mem_reg *reg = &iser_task->rdma_reg[cmd_dir];
  1116          struct iser_fr_desc *desc = reg->mem_h;
  1117          unsigned long sector_size = iser_task->sc->device->sector_size;
  1118          struct ib_mr_status mr_status;
  1119          int ret;
  1120  
  1121          if (desc && desc->pi_ctx->sig_protected) {
  1122                  desc->pi_ctx->sig_protected = 0;
  1123                  ret = ib_check_mr_status(desc->pi_ctx->sig_mr,
  1124                                           IB_MR_CHECK_SIG_STATUS, &mr_status);
  1125                  if (ret) {
  1126                          pr_err("ib_check_mr_status failed, ret %d\n", ret);
  1127                          goto err;

But we have an error path where it's not initialized.

  1128                  }
  1129  
  1130                  if (mr_status.fail_status & IB_MR_CHECK_SIG_STATUS) {
  1131                          sector_t sector_off = mr_status.sig_err.sig_err_offset;
  1132  
  1133                          sector_div(sector_off, sector_size + 8);
  1134                          *sector = scsi_get_lba(iser_task->sc) + sector_off;
  1135  
  1136                          pr_err("PI error found type %d at sector %llx "
  1137                                 "expected %x vs actual %x\n",
  1138                                 mr_status.sig_err.err_type,
  1139                                 (unsigned long long)*sector,
  1140                                 mr_status.sig_err.expected,
  1141                                 mr_status.sig_err.actual);
  1142  
  1143                          switch (mr_status.sig_err.err_type) {
  1144                          case IB_SIG_BAD_GUARD:
  1145                                  return 0x1;
  1146                          case IB_SIG_BAD_REFTAG:
  1147                                  return 0x3;
  1148                          case IB_SIG_BAD_APPTAG:
  1149                                  return 0x2;
  1150                          }
  1151                  }
  1152          }
  1153  
  1154          return 0;
  1155  err:
  1156          /* Not alot we can do here, return ambiguous guard error */
  1157          return 0x1;
  1158  }

regards,
dan carpenter

You received this message because you are subscribed to the Google Groups "open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected]
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/[email protected]
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.