* re: libiscsi: Use scsi helper to set information descriptor
@ 2016-04-13 13:14 Dan Carpenter
2016-04-13 14:53 ` Sagi Grimberg
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2016-04-13 13:14 UTC (permalink / raw)
To: sagig; +Cc: open-iscsi, linux-scsi
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, §or);
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: libiscsi: Use scsi helper to set information descriptor
2016-04-13 13:14 libiscsi: Use scsi helper to set information descriptor Dan Carpenter
@ 2016-04-13 14:53 ` Sagi Grimberg
2016-04-13 16:30 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Sagi Grimberg @ 2016-04-13 14:53 UTC (permalink / raw)
To: Dan Carpenter; +Cc: open-iscsi, linux-scsi
Hey Dan,
> 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, §or);
>
> 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
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)?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: libiscsi: Use scsi helper to set information descriptor
2016-04-13 14:53 ` Sagi Grimberg
@ 2016-04-13 16:30 ` Dan Carpenter
2016-05-04 11:45 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2016-04-13 16:30 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: open-iscsi, linux-scsi
On Wed, Apr 13, 2016 at 05:53:08PM +0300, Sagi Grimberg wrote:
> Hey Dan,
>
> >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, §or);
> >
> >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
>
> iscsi transports that implement this callout are expected
> to set the sector which is passed by reference.
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.
>
> would it make the checker happy if we set sector to 0 before
> calling check_protection (although it's not needed by no means)?
It looks for if there is any possible way that it could be uninitialized
so that would solve the problem.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: libiscsi: Use scsi helper to set information descriptor
2016-04-13 16:30 ` Dan Carpenter
@ 2016-05-04 11:45 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2016-05-04 11:45 UTC (permalink / raw)
To: Sagi Grimberg
Cc: open-iscsi-/JYPxA39Uh5TLH3MbocFFw, linux-scsi-u79uwXL29TY76Z2rM5mHXA
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/Ez6ZCGd0@public.gmane.org
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-05-04 11:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-13 13:14 libiscsi: Use scsi helper to set information descriptor Dan Carpenter
2016-04-13 14:53 ` Sagi Grimberg
2016-04-13 16:30 ` Dan Carpenter
2016-05-04 11:45 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.