[PATCH v3] scsi: libsas: fix length error in sas_smp_handler()
The return value of smp_execute_task_sg() is the untransferred residual,
but bsg_job_done() requires the length of payload received. This makes
SMP passthrough commands from userland by sg ioctl to libsas get a wrong
response. The userland tools such as smp_utils failed becuase of these
wrong responses:
~#smp_discover /dev/bsg/expander-2\:13
response too short, len=0
~#smp_discover /dev/bsg/expander-2\:134
response too short, len=0
Fix this by passing the actual received length to bsg_job_done(). And if
smp_execute_task_sg() returns 0, this means received length is exactly
the buffer length.
Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for SMP
passthrough")
Reported-and-tested-by: chenqilin
Signed-off-by: Jason Yan
CC: Christoph Hellwig
---
drivers/scsi/libsas/sas_expander.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..6c40ecc 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2143,7 +2143,7 @@ void sas_smp_handler(struct bsg_job *job, struct
Scsi_Host *shost,
struct sas_rphy *rphy)
{
struct domain_device *dev;
- unsigned int reslen = 0;
+ unsigned int rcvlen = 0;
int ret = -EINVAL;
/* no rphy means no smp target support (ie aic94xx host) */
@@ -2177,12 +2177,12 @@ void sas_smp_handler(struct bsg_job *job, struct
Scsi_Host *shost,
ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
job->reply_payload.sg_list);
- if (ret > 0) {
- /* positive number is the untransferred residual */
- reslen = ret;
+ if (ret >= 0) {
+ /* bsg_job_done() requires the length received */
+ rcvlen = job->reply_payload.payload_len - ret;
ret = 0;
}
out:
- bsg_job_done(job, ret, reslen);
+ bsg_job_done(job, ret, rcvlen);
}
--
2.9.5
Re: [bug report] scsi: arcmsr: Add a function to set date and time to firmware
Dan,
Yeah, secs is never be negative. This checking is redundant and have
to be removed.
Thanks Colin Ian King has done this patch for me on [next].
Thanks,
Ching
2017-12-09 20:47 GMT+08:00 Dan Carpenter :
> Hello Ching Huang,
>
> The patch b416c099472a: "scsi: arcmsr: Add a function to set date and
> time to firmware" from Dec 5, 2017, leads to the following static
> checker warning:
>
> drivers/scsi/arcmsr/arcmsr_hba.c:3682 arcmsr_set_iop_datetime()
> warn: unsigned 'secs' is never less than zero.
>
> drivers/scsi/arcmsr/arcmsr_hba.c
> 3658 static void arcmsr_set_iop_datetime(struct timer_list *t)
> 3659 {
> 3660 struct AdapterControlBlock *pacb = from_timer(pacb, t,
> refresh_timer);
> 3661 unsigned int days, j, i, a, b, c, d, e, m, year, mon, day,
> hour, min, sec, secs, next_time;
>
> ^^^
>
> 3662 struct timeval tv;
> 3663 union {
> 3664 struct {
> 3665 uint16_tsignature;
> 3666 uint8_t year;
> 3667 uint8_t month;
> 3668 uint8_t date;
> 3669 uint8_t hour;
> 3670 uint8_t minute;
> 3671 uint8_t second;
> 3672 } a;
> 3673 struct {
> 3674 uint32_tmsg_time[2];
> 3675 } b;
> 3676 } datetime;
> 3677
> 3678 do_gettimeofday(&tv);
> 3679 secs = (u32)(tv.tv_sec - (sys_tz.tz_minuteswest * 60));
> 3680 days = secs / 86400;
> 3681 secs = secs - 86400 * days;
> 3682 if (secs < 0) {
>
> Not possible.
>
> 3683 days = days - 1;
> 3684 secs = secs + 86400;
> 3685 }
> 3686 j = days / 146097;
>
>
> regards,
> dan carpenter
Re: [PATCH][next] scsi: arcmsr: remove redundant check for secs < 0
Colin,
You are right. That's checking is redundant. secs is never be negative.
Thanks for your correction and patch.
Regards,
Ching
2017-12-09 8:34 GMT+08:00 Colin King :
> From: Colin Ian King
>
> The check for secs being less than zero is redundant for two reasons.
> Firstly, secs is unsigned so the check is always going to be false.
> Secondly, if secs was signed the proceeding calculation of secs is
> never going to be negative. Hence we can remove this redundant check
> and day and secs re-adjustment.
>
> Detected by static analysis with smatch:
> arcmsr_set_iop_datetime() warn: unsigned 'secs' is never less than zero.
>
> Signed-off-by: Colin Ian King
> ---
> drivers/scsi/arcmsr/arcmsr_hba.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 0707a60bf5c0..e4258b69f4be 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -3679,10 +3679,6 @@ static void arcmsr_set_iop_datetime(struct timer_list
> *t)
> secs = (u32)(tv.tv_sec - (sys_tz.tz_minuteswest * 60));
> days = secs / 86400;
> secs = secs - 86400 * days;
> - if (secs < 0) {
> - days = days - 1;
> - secs = secs + 86400;
> - }
> j = days / 146097;
> i = days - 146097 * j;
> a = i + 719468;
> --
> 2.14.1
>
Re: [PATCH] sbp-target: Delete an error message for a failed memory allocation in three functions
On 10/12/2017 19:10, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sun, 10 Dec 2017 19:54:11 +0100 > > Omit an extra message for a memory allocation failure in these functions. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring [snip] Looks good to me. Acked-by: Chris Boot Thanks, Chris -- Chris Boot bo...@boo.tc
[PATCH 1/1] scsi: fnic: add a space after %p in printf format
fnic_fcpio_icmnd_cmpl_handler() displays the value of sc with: FNIC_SCSI_DBG(KERN_INFO... "... sc = 0x%p" "scsi_status ..." ... As the literal strings get merged, the function uses %ps instead of the intended raw %p format. Fix this by inserting a space. Signed-off-by: Nicolas Iooss --- drivers/scsi/fnic/fnic_scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c index 242e2ee494a1..8cbd3c9f0b4c 100644 --- a/drivers/scsi/fnic/fnic_scsi.c +++ b/drivers/scsi/fnic/fnic_scsi.c @@ -906,7 +906,7 @@ static void fnic_fcpio_icmnd_cmpl_handler(struct fnic *fnic, FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host, "icmnd_cmpl abts pending " - "hdr status = %s tag = 0x%x sc = 0x%p" + "hdr status = %s tag = 0x%x sc = 0x%p " "scsi_status = %x residual = %d\n", fnic_fcpio_status_to_str(hdr_status), id, sc, -- 2.15.0
[PATCH] sbp-target: Delete an error message for a failed memory allocation in three functions
From: Markus Elfring
Date: Sun, 10 Dec 2017 19:54:11 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/target/sbp/sbp_target.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
index e5c3e5f827d0..fb1003921d85 100644
--- a/drivers/target/sbp/sbp_target.c
+++ b/drivers/target/sbp/sbp_target.c
@@ -201,10 +201,9 @@ static struct sbp_session *sbp_session_create(
snprintf(guid_str, sizeof(guid_str), "%016llx", guid);
sess = kmalloc(sizeof(*sess), GFP_KERNEL);
- if (!sess) {
- pr_err("failed to allocate session descriptor\n");
+ if (!sess)
return ERR_PTR(-ENOMEM);
- }
+
spin_lock_init(&sess->lock);
INIT_LIST_HEAD(&sess->login_list);
INIT_DELAYED_WORK(&sess->maint_work, session_maintenance_work);
@@ -2029,10 +2028,8 @@ static struct se_portal_group *sbp_make_tpg(
}
tpg = kzalloc(sizeof(*tpg), GFP_KERNEL);
- if (!tpg) {
- pr_err("Unable to allocate struct sbp_tpg\n");
+ if (!tpg)
return ERR_PTR(-ENOMEM);
- }
tpg->tport = tport;
tpg->tport_tpgt = tpgt;
@@ -2088,10 +2085,8 @@ static struct se_wwn *sbp_make_tport(
return ERR_PTR(-EINVAL);
tport = kzalloc(sizeof(*tport), GFP_KERNEL);
- if (!tport) {
- pr_err("Unable to allocate struct sbp_tport\n");
+ if (!tport)
return ERR_PTR(-ENOMEM);
- }
tport->guid = guid;
sbp_format_wwn(tport->tport_name, SBP_NAMELEN, guid);
--
2.15.1
[PATCH] documentation: add scsi_common.c to SCSI driver-api
From: Randy Dunlap Add exported functions from scsi_common.c to the SCSI driver API documentation. Signed-off-by: Randy Dunlap Cc: Nicholas Bellinger --- Documentation/driver-api/scsi.rst |8 1 file changed, 8 insertions(+) --- lnx-415-rc2.orig/Documentation/driver-api/scsi.rst +++ lnx-415-rc2/Documentation/driver-api/scsi.rst @@ -224,6 +224,14 @@ mid to lowlevel SCSI driver interface .. kernel-doc:: drivers/scsi/hosts.c :export: +drivers/scsi/scsi_common.c +~~ + +general support functions + +.. kernel-doc:: drivers/scsi/scsi_common.c + :export: + Transport classes -
[PATCH] scsi: doc. fixes to scsi_common.c
From: Randy Dunlap
Clean up some comment typos and fix some errors in documentation.
Signed-off-by: Randy Dunlap
Cc: Nicholas Bellinger
Cc: Sagi Grimberg
Cc: Bart Van Assche
---
drivers/scsi/scsi_common.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
--- lnx-415-rc2.orig/drivers/scsi/scsi_common.c
+++ lnx-415-rc2/drivers/scsi/scsi_common.c
@@ -12,7 +12,7 @@
/* NB: These are exposed through /proc/scsi/scsi and form part of the ABI.
* You may not alter any existing entry (although adding new ones is
- * encouraged once assigned by ANSI/INCITS T10
+ * encouraged once assigned by ANSI/INCITS T10).
*/
static const char *const scsi_device_types[] = {
"Direct-Access",
@@ -39,7 +39,7 @@ static const char *const scsi_device_typ
};
/**
- * scsi_device_type - Return 17 char string indicating device type.
+ * scsi_device_type - Return 17-char string indicating device type.
* @type: type number to look up
*/
const char *scsi_device_type(unsigned type)
@@ -59,7 +59,7 @@ EXPORT_SYMBOL(scsi_device_type);
* @scsilun: struct scsi_lun to be converted.
*
* Description:
- * Convert @scsilun from a struct scsi_lun to a four byte host byte-ordered
+ * Convert @scsilun from a struct scsi_lun to a four-byte host byte-ordered
* integer, and return the result. The caller must check for
* truncation before using this function.
*
@@ -98,7 +98,7 @@ EXPORT_SYMBOL(scsilun_to_int);
* back into the lun value.
*
* Notes:
- * Given an integer : 0x0b03d204, this function returns a
+ * Given an integer : 0x0b03d204, this function returns a
* struct scsi_lun of: d2 04 0b 03 00 00 00 00
*
*/
@@ -221,7 +221,7 @@ EXPORT_SYMBOL(scsi_sense_desc_find);
/**
* scsi_build_sense_buffer - build sense data in a buffer
- * @desc: Sense format (non zero == descriptor format,
+ * @desc: Sense format (non-zero == descriptor format,
* 0 == fixed format)
* @buf: Where to build sense data
* @key: Sense key
@@ -255,7 +255,7 @@ EXPORT_SYMBOL(scsi_build_sense_buffer);
* @info: 64-bit information value to be set
*
* Return value:
- * 0 on success or EINVAL for invalid sense buffer length
+ * 0 on success or -EINVAL for invalid sense buffer length
**/
int scsi_set_sense_information(u8 *buf, int buf_len, u64 info)
{
@@ -305,7 +305,7 @@ EXPORT_SYMBOL(scsi_set_sense_information
* @cd:command/data bit
*
* Return value:
- * 0 on success or EINVAL for invalid sense buffer length
+ * 0 on success or -EINVAL for invalid sense buffer length
*/
int scsi_set_sense_field_pointer(u8 *buf, int buf_len, u16 fp, u8 bp, bool cd)
{
Re: [PATCH] sd: Increase SCSI disk probing concurrency
On 12/08/2017 06:21 PM, Bart Van Assche wrote: > The scsi_sd_probe_domain allows to wait until all disk-probing > activity has finished system-wide. This slows down SCSI host removal > that occurs concurrently with SCSI disk probing because sd_remove() > waits on scsi_sd_probe_domain. Additionally, since each function that > waits on scsi_sd_probe_domain specifies for which disk to wait until > probing has finished, replace waiting on scsi_sd_probe_domain by > waiting until probing for a specific disk has finished. Introduce a > .sync() function pointer in struct scsi_driver to make it possible > for the SCSI power management code to wait until probing of a > specific disk has finished. > > Signed-off-by: Bart Van Assche > Cc: Christoph Hellwig > Cc: Hannes Reinecke > Cc: Johannes Thumshirn > --- > drivers/scsi/scsi.c| 5 - > drivers/scsi/scsi_pm.c | 6 -- > drivers/scsi/scsi_priv.h | 1 - > drivers/scsi/sd.c | 26 +- > drivers/scsi/sd.h | 1 + > include/scsi/scsi_driver.h | 1 + > 6 files changed, 27 insertions(+), 13 deletions(-) > You know what, I have been working on a similar patch for quite some time now; however, I've taken the simpler approach of not using async_synchronize_full_domain() but rather async_synchronize_cookie(), which makes for a simpler patch :-) However, in doing so I have encountered several issues which have been exposed by that; the most trivial one being that del_gendisk() doesn't check if GENHD_FL_UP is set, so it'll crash if sd_remove is called before sd_async_probe() is run. The other one is an imbalance between sd_probe and sd_remove; when sd_probe_async() is called _after_ scsi_device_remove_device() (which it will as the synchronization is only after device_del() has been called) it will trip over non-existent sysfs directories in add_disk(). So one need to short-circuit sd_probe_async() for devices in SDEV_CANCEL or SDEV_DEL. However, I'm still waiting for a final confirmation on the latter issue, hence I haven't posted the patchset. If there's interest I can post them, though. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.com +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
