Re: [patch] [SCSI] mpt3sas: move dereference under check
On Thu, Mar 14, 2013 at 06:52:35PM +0530, Reddy, Sreekanth wrote:
Hi Dan Carpenter,
While analyzing this patch, I have added some debugging prints to
print the address referenced by the IOC-sense_dma_pool before and
after pci_pool_free() API and I have observed that the address
referenced by this pointer is same before and after calling
pci_pool_free() API. Please let me know if you have any case
where you have seen ioc-sense_dma_pool is dereferenced after
calling pci_pool_free API.
And we are checking this ioc-sense_dma_pool pointer to NULL value
for safe side even it is not assigned to NULL value anywhere in
the driver code.
Thanks for looking at this.
I hope you don't mind, but I've added linux-scsi back to the CC.
This is a static checker which is complaining that the code is not
checking for NULL consistently. If ioc-sense_dma_pool is NULL
then we will crash inside pci_pool_free(). The NULL dereference is
on the line where we do:
mm/dmapool.c
391 void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t dma)
392 {
393 struct dma_page *page;
394 unsigned long flags;
395 unsigned int offset;
396
397 spin_lock_irqsave(pool-lock, flags);
^^^
This dereference will cause a crash because we call pci_pool_free()
without testing for NULL.
398 page = pool_find_page(pool, dma);
In other words:
drivers/scsi/mpt3sas/mpt3sas_base.c
2481 if (ioc-sense) {
2482 pci_pool_free(ioc-sense_dma_pool, ioc-sense,
ioc-sense_dma);
^
We crash on this line before we reach the NULL test on the next
line.
2483 if (ioc-sense_dma_pool)
2484 pci_pool_destroy(ioc-sense_dma_pool);
I've looked at the code, and you're right that if (ioc-sense) {
is non-NULL that means -sense_dma_pool is non-NULL. ioc-sense is
allocated from the DMA pool.
I would like to just remove the test and silence the warning
message.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] csiostor: off by one error
We need to store PROTO_ERR_IMPL_LOGO (26) things here, but the
first element isn't used so the array should have 27 elements.
This matches fwevt_to_rnevt[] which has 27 elements.
The patch solves a Smatch static checker warning on my system:
drivers/scsi/csiostor/csio_rnode.c:880 csio_rnode_fwevt_handler()
error: buffer overflow '(rn)-stats.n_evt_fw' 26 = 26
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
This goes on top of d69630e8a4222 csiostor: Header file
modifications for chip support and bug fixes. That patch is in
linux-next but I don't know which tree it came from. It's not
the scsi for-next tree.
diff --git a/drivers/scsi/csiostor/csio_rnode.h
b/drivers/scsi/csiostor/csio_rnode.h
index 6594009..4334342 100644
--- a/drivers/scsi/csiostor/csio_rnode.h
+++ b/drivers/scsi/csiostor/csio_rnode.h
@@ -63,7 +63,7 @@ struct csio_rnode_stats {
uint32_tn_err_nomem;/* error nomem */
uint32_tn_evt_unexp;/* unexpected event */
uint32_tn_evt_drop; /* unexpected event */
- uint32_tn_evt_fw[PROTO_ERR_IMPL_LOGO]; /* fw events */
+ uint32_tn_evt_fw[PROTO_ERR_IMPL_LOGO + 1]; /* fw events */
enum csio_rn_ev n_evt_sm[CSIO_RNFE_MAX_EVENT]; /* State m/c events */
uint32_tn_lun_rst; /* Number of resets of
* of LUNs under this
diff --git a/drivers/scsi/csiostor/csio_lnode.h
b/drivers/scsi/csiostor/csio_lnode.h
index 0f9c041..372a67d 100644
--- a/drivers/scsi/csiostor/csio_lnode.h
+++ b/drivers/scsi/csiostor/csio_lnode.h
@@ -114,7 +114,7 @@ struct csio_lnode_stats {
uint32_tn_rnode_match; /* matched rnode */
uint32_tn_dev_loss_tmo; /* Device loss timeout */
uint32_tn_fdmi_err; /* fdmi err */
- uint32_tn_evt_fw[PROTO_ERR_IMPL_LOGO]; /* fw events */
+ uint32_tn_evt_fw[PROTO_ERR_IMPL_LOGO + 1]; /* fw events */
enum csio_ln_ev n_evt_sm[CSIO_LNE_MAX_EVENT]; /* State m/c events */
uint32_tn_rnode_alloc; /* rnode allocated */
uint32_tn_rnode_free; /* rnode freed */
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] qla4xxx: Add flash node mgmt support
Hi Adheer,
The patch 1e9e2be3ee03: [SCSI] qla4xxx: Add flash node mgmt support
from Mar 22, 2013, has several endian bugs.
drivers/scsi/qla4xxx/ql4_os.c
2217 fw_ddb_entry-tgt_portal_grp = cpu_to_le16(sess-tpgt);
2218 fw_ddb_entry-mss = cpu_to_le16(conn-max_segment_size);
2219 fw_ddb_entry-tcp_xmt_wsf = cpu_to_le16(conn-tcp_xmit_wsf);
^^^
This is u8.
2220 fw_ddb_entry-tcp_rcv_wsf = cpu_to_le16(conn-tcp_recv_wsf);
^^^
This is u8.
2221 fw_ddb_entry-ipv4_tos = conn-ipv4_tos;
fw_ddb_entry-ipv6_flow_lbl =
cpu_to_le16(conn-ipv6_flow_label);
2223 fw_ddb_entry-ka_timeout = cpu_to_le16(conn-keepalive_timeout);
2224 fw_ddb_entry-lcl_port = cpu_to_le16(conn-local_port);
2225 fw_ddb_entry-stat_sn = cpu_to_le16(conn-statsn);
^^^
This is u32.
2226 fw_ddb_entry-exp_stat_sn = cpu_to_le16(conn-exp_statsn);
^^^
This is u32.
2227 fw_ddb_entry-ddb_link =
cpu_to_le16(sess-discovery_parent_type);
2228 fw_ddb_entry-chap_tbl_idx = cpu_to_le16(sess-chap_out_idx);
2229 fw_ddb_entry-tsid = cpu_to_le16(sess-tsid);
Theoretically these should have been caught by Sparse:
http://lwn.net/Articles/205624/
But unfortunately, Sparse hits an error parsing the
external_hw_config_reg union because it uses bitfields as part of __le32
data. After you hit a Sparse error then it doesn't bother to print
warnings. This is arguably a UI problem in Sparse and it took me
forever to figure out why the warnings weren't being printed. :/
If I changed the external_hw_config_reg to use u32 instead of __le32
then Sparse gives the max number of warnings. I'm not sure that that's
the right thing to do. Are those bitfields actually used? Maybe we
should just delete it.
Anyway, I've attached the warnings below.
regards,
dan carpenter
stdin:1223:2: warning: #warning syscall finit_module not implemented [-Wcpp]
devel/drivers/scsi/qla4xxx/ql4_os.c:1370:28: warning: incorrect type in
assignment (different base types)
devel/drivers/scsi/qla4xxx/ql4_os.c:1370:28:expected unsigned int
[unsigned] [usertype] cookie
devel/drivers/scsi/qla4xxx/ql4_os.c:1370:28:got restricted __le32
[usertype] noident
devel/drivers/scsi/qla4xxx/ql4_os.c:1890:21: warning: incorrect type in
assignment (different base types)
devel/drivers/scsi/qla4xxx/ql4_os.c:1890:21:expected restricted itt_t
[usertype] itt
devel/drivers/scsi/qla4xxx/ql4_os.c:1890:21:got unsigned int [unsigned]
[usertype] handle
devel/drivers/scsi/qla4xxx/ql4_os.c:901:32: warning: cast to restricted __le64
devel/drivers/scsi/qla4xxx/ql4_os.c:902:32: warning: cast to restricted __le64
devel/drivers/scsi/qla4xxx/ql4_os.c:904:29: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:905:31: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:906:30: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:907:29: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:908:28: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:909:31: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:910:30: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:911:29: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:913:29: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:914:31: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:915:30: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:916:31: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:917:30: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:919:25: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:920:27: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:921:29: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:922:27: warning: cast to restricted __le32
devel/drivers/scsi/qla4xxx/ql4_os.c:502:21: warning: restricted __le16 degrades
to integer
devel/drivers/scsi/qla4xxx/ql4_os.c:627:9: warning: cast to restricted __le16
devel/drivers/scsi/qla4xxx/ql4_os.c:630:13: warning: cast to restricted __le16
devel/drivers/scsi/qla4xxx/ql4_os.c:635:28: warning: incorrect type in
assignment (different base types)
devel/drivers/scsi/qla4xxx/ql4_os.c:635:28:expected unsigned short
[unsigned] [usertype] cookie
devel/drivers/scsi/qla4xxx/ql4_os.c:635:28:got restricted __le16 [usertype]
noident
devel/drivers/scsi/qla4xxx/ql4_os.c:1126:53: warning: invalid assignment: =
devel/drivers/scsi/qla4xxx/ql4_os.c:1126:53:left side has type unsigned
short
devel/drivers/scsi/qla4xxx/ql4_os.c:1126:53:right side has type restricted
__le16
devel/drivers/scsi
re: [SCSI] qla2xxx: Enhancements to support ISPFx00.
Hello Giridhar Malavali, The patch 8ae6d9c7eb10: [SCSI] qla2xxx: Enhancements to support ISPFx00. from Mar 28, 2013, leads to a bunch of Sparse endian warnings. See: http://lwn.net/Articles/205624/ Most of them are just because the code hasn't been annotated but some look like real bugs like on line 3288. I've included the warnings below. regards, dan carpenter stdin:1223:2: warning: #warning syscall finit_module not implemented [-Wcpp] drivers/scsi/qla2xxx/qla_mr.c:55:21: warning: restricted pci_channel_state_t degrades to integer drivers/scsi/qla2xxx/qla_mr.c:55:37: warning: restricted pci_channel_state_t degrades to integer drivers/scsi/qla2xxx/qla_mr.c:643:35: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_mr.c:643:35:expected unsigned short [unsigned] [usertype] request_q_outpointer drivers/scsi/qla2xxx/qla_mr.c:643:35:got restricted __le16 [usertype] noident drivers/scsi/qla2xxx/qla_mr.c:644:35: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_mr.c:644:35:expected unsigned short [unsigned] [usertype] response_q_inpointer drivers/scsi/qla2xxx/qla_mr.c:644:35:got restricted __le16 [usertype] noident drivers/scsi/qla2xxx/qla_mr.c:645:31: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_mr.c:645:31:expected unsigned short [unsigned] [usertype] request_q_length drivers/scsi/qla2xxx/qla_mr.c:645:31:got restricted __le16 [usertype] noident drivers/scsi/qla2xxx/qla_mr.c:646:32: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_mr.c:646:32:expected unsigned short [unsigned] [usertype] response_q_length drivers/scsi/qla2xxx/qla_mr.c:646:32:got restricted __le16 [usertype] noident drivers/scsi/qla2xxx/qla_mr.c:647:35: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_mr.c:647:35:expected unsigned int [unsigned] noident drivers/scsi/qla2xxx/qla_mr.c:647:35:got restricted __le32 [usertype] noident drivers/scsi/qla2xxx/qla_mr.c:648:35: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_mr.c:648:35:expected unsigned int [unsigned] noident drivers/scsi/qla2xxx/qla_mr.c:648:35:got restricted __le32 [usertype] noident drivers/scsi/qla2xxx/qla_mr.c:649:36: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_mr.c:649:36:expected unsigned int [unsigned] noident drivers/scsi/qla2xxx/qla_mr.c:649:36:got restricted __le32 [usertype] noident drivers/scsi/qla2xxx/qla_mr.c:650:36: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_mr.c:650:36:expected unsigned int [unsigned] noident drivers/scsi/qla2xxx/qla_mr.c:650:36:got restricted __le32 [usertype] noident drivers/scsi/qla2xxx/qla_mr.c:883:22: warning: cast removes address space of expression drivers/scsi/qla2xxx/qla_mr.c:898:22: warning: cast removes address space of expression drivers/scsi/qla2xxx/qla_mr.c:1424:17: warning: incorrect type in argument 2 (different address spaces) drivers/scsi/qla2xxx/qla_mr.c:1424:17:expected void volatile [noderef] asn:2*addr drivers/scsi/qla2xxx/qla_mr.c:1424:17:got unsigned int *noident drivers/scsi/qla2xxx/qla_mr.c:2200:34: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c::49: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c:2223:47: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c:2224:45: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c:2227:29: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c:2298:23: warning: cast to restricted __le16 drivers/scsi/qla2xxx/qla_mr.c:2299:23: warning: cast to restricted __le16 drivers/scsi/qla2xxx/qla_mr.c:2353:29: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c:2355:29: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c:2357:32: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c:2652:26: warning: cast to restricted __le32 drivers/scsi/qla2xxx/qla_mr.c:2720:16: warning: incorrect type in argument 1 (different address spaces) drivers/scsi/qla2xxx/qla_mr.c:2720:16:expected void const volatile [noderef] asn:2*addr drivers/scsi/qla2xxx/qla_mr.c:2720:16:got unsigned int *noident drivers/scsi/qla2xxx/qla_mr.c:2723:45: warning: incorrect type in argument 2 (different address spaces) drivers/scsi/qla2xxx/qla_mr.c:2723:45:expected void const volatile [noderef] asn:2*src drivers/scsi/qla2xxx/qla_mr.c:2723:45:got struct response_t [usertype] *[assigned] lptr drivers/scsi/qla2xxx/qla_mr.c:2774:17: warning: incorrect type in argument 2 (different address spaces) drivers/scsi/qla2xxx/qla_mr.c:2774:17:expected void volatile [noderef] asn:2*addr drivers/scsi/qla2xxx/qla_mr.c:2774:17:got unsigned int *noident drivers/scsi/qla2xxx/qla_mr.c:3152:29: warning: incorrect
[patch] [SCSI] megaraid_sas: release lock on error path
We should unlock here before returning. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 9d53540..eb2385f 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -4931,11 +4931,12 @@ static int megasas_mgmt_ioctl_fw(struct file *file, unsigned long arg) printk(KERN_ERR megaraid_sas: timed out while waiting for HBA to recover\n); error = -ENODEV; - goto out_kfree_ioc; + goto out_up; } spin_unlock_irqrestore(instance-hba_lock, flags); error = megasas_mgmt_fw_ioctl(instance, user_ioc, ioc); + out_up: up(instance-ioctl_sem); out_kfree_ioc: -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] mvsas: fix a sparse annotation
It doesn't make sense to label a 22 bit bitfield as __le32. It just
causes a Sparse error.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/mvsas/mv_94xx.h b/drivers/scsi/mvsas/mv_94xx.h
index 487aa6f..b11f90d 100644
--- a/drivers/scsi/mvsas/mv_94xx.h
+++ b/drivers/scsi/mvsas/mv_94xx.h
@@ -220,7 +220,7 @@ union reg_phy_cfg {
struct mvs_prd_imt {
#ifndef __BIG_ENDIAN
- __le32 len:22;
+ u32 len:22;
u8 _r_a:2;
u8 misc_ctl:4;
u8 inter_sel:4;
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] libosd: remover duplicate __bitwise annotation
__be32 is already a __bitwise type so we don't need the second __bitwise
here. It causes a Sparse error:
include/scsi/osd_protocol.h:110:26: error: invalid modifier
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/include/scsi/osd_protocol.h b/include/scsi/osd_protocol.h
index a6026da..25ac628 100644
--- a/include/scsi/osd_protocol.h
+++ b/include/scsi/osd_protocol.h
@@ -107,7 +107,7 @@ enum osd_attributes_mode {
* int exponent: 04;
* }
*/
-typedef __be32 __bitwise osd_cdb_offset;
+typedef __be32 osd_cdb_offset;
enum {
OSD_OFFSET_UNUSED = 0x,
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] ufs: add support for query requests
Hello Dolev Raviv,
This is a semi-automatic email about new static checker warnings.
The patch 3aee47c623a3: [SCSI] ufs: add support for query requests
from May 2, 2013, leads to the following Smatch complaint:
drivers/scsi/ufs/ufshcd.c:723 ufshcd_query_request()
error: we previously assumed 'hba' could be null (see line 722)
drivers/scsi/ufs/ufshcd.c
721
722 if (!hba || !query || !response) {
New check.
723 dev_err(hba-dev,
New dereference.
724 %s: NULL pointer hba = %p, query = %p response
= %p\n,
725 __func__, hba, query, response);
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] pm80xx: NCQ error handling changes
Hello Sakthivel K,
This is a semi-automatic email about new static checker warnings.
The patch 1cde970c1025: [SCSI] pm80xx: NCQ error handling changes
from Mar 19, 2013, leads to the following Smatch complaint:
drivers/scsi/pm8001/pm8001_hwi.c:4393 pm8001_chip_sata_req()
warn: variable dereferenced before check 'pm8001_ha_dev' (see line
4362)
drivers/scsi/pm8001/pm8001_hwi.c
4361 sata_cmd.tag = cpu_to_le32(tag);
4362 sata_cmd.device_id = cpu_to_le32(pm8001_ha_dev-device_id);
^
Old dereference.
4363 sata_cmd.data_len = cpu_to_le32(task-total_xfer_len);
4364 sata_cmd.ncqtag_atap_dir_m =
4365 cpu_to_le32(((ncg_tag 0xff)16)|((ATAP 0x3f)
10) | dir);
4366 sata_cmd.sata_fis = task-ata_task.fis;
4367 if (likely(!task-ata_task.device_control_reg_update))
4368 sata_cmd.sata_fis.flags |= 0x80;/* C=1: update ATA cmd
reg */
4369 sata_cmd.sata_fis.flags = 0xF0;/* PM_PORT field shall be 0 */
4370 /* fill in PRD (scatter/gather) table, if any */
4371 if (task-num_scatter 1) {
4372 pm8001_chip_make_sg(task-scatter, ccb-n_elem,
ccb-buf_prd);
4373 phys_addr = ccb-ccb_dma_handle +
4374 offsetof(struct pm8001_ccb_info,
buf_prd[0]);
4375 sata_cmd.addr_low = lower_32_bits(phys_addr);
4376 sata_cmd.addr_high = upper_32_bits(phys_addr);
4377 sata_cmd.esgl = cpu_to_le32(1 31);
4378 } else if (task-num_scatter == 1) {
4379 u64 dma_addr = sg_dma_address(task-scatter);
4380 sata_cmd.addr_low = lower_32_bits(dma_addr);
4381 sata_cmd.addr_high = upper_32_bits(dma_addr);
4382 sata_cmd.len = cpu_to_le32(task-total_xfer_len);
4383 sata_cmd.esgl = 0;
4384 } else if (task-num_scatter == 0) {
4385 sata_cmd.addr_low = 0;
4386 sata_cmd.addr_high = 0;
4387 sata_cmd.len = cpu_to_le32(task-total_xfer_len);
4388 sata_cmd.esgl = 0;
4389 }
4390
4391 /* Check for read log for failed drive and return */
4392 if (sata_cmd.sata_fis.command == 0x2f) {
4393 if (pm8001_ha_dev ((pm8001_ha_dev-id
NCQ_READ_LOG_FLAG) ||
^
New check. Probably this check is unneeded and can be removed.
4394 (pm8001_ha_dev-id NCQ_ABORT_ALL_FLAG) ||
4395 (pm8001_ha_dev-id NCQ_2ND_RLE_FLAG))) {
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] pm80xx: missing break statement in mpi_sata_completion()
Smatch discovered a missing break statement here. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 670998a..6c0470e 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -2021,6 +2021,7 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb) ts-resp = SAS_TASK_COMPLETE; ts-stat = SAS_OPEN_REJECT; ts-open_rej_reason = SAS_OREJ_RSVD_RETRY; + break; default: PM8001_IO_DBG(pm8001_ha, pm8001_printk(Unknown status 0x%x\n, status)); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] pm80xx: remove unneeded NULL check
Coccinelle complains about the inconsistent NULL checking on t. It
turns out the check isn't needed because we verified that t is
non-NULL at the start of the function.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 69dd49c..683df42 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -3740,7 +3740,7 @@ int pm8001_mpi_task_abort_resp(struct pm8001_hba_info
*pm8001_ha, void *piomb)
pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
mb();
- if ((pm8001_dev-id NCQ_ABORT_ALL_FLAG) t) {
+ if (pm8001_dev-id NCQ_ABORT_ALL_FLAG) {
pm8001_tag_free(pm8001_ha, tag);
sas_free_task(t);
/* clear the flag */
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS
On Thu, May 16, 2013 at 05:21:19AM -0700, K. Y. Srinivasan wrote: Increase the value of STORVSC_MAX_IO_REQUESTS to 200 requests. The current ringbuffer size can support this higher value. The ringbuffer size is a module parameter so it's odd to talk about the current size. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS
On Thu, May 16, 2013 at 01:37:41PM +, KY Srinivasan wrote: -Original Message- From: Dan Carpenter [mailto:dan.carpen...@oracle.com] Sent: Thursday, May 16, 2013 8:02 AM To: KY Srinivasan Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; oher...@suse.com; jbottom...@parallels.com; h...@infradead.org; linux-scsi@vger.kernel.org; a...@canonical.com; jasow...@redhat.com Subject: Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS On Thu, May 16, 2013 at 05:21:19AM -0700, K. Y. Srinivasan wrote: Increase the value of STORVSC_MAX_IO_REQUESTS to 200 requests. The current ringbuffer size can support this higher value. The ringbuffer size is a module parameter so it's odd to talk about the current size. While the ringbuffer size is a module parameter; there is a default value. The current size refers to the default. Your comment applies to the current value (of 128) as well in that it is possible for somebody to load this driver with a ringbuffer size that could not support the value of 128. If this is the case, we fail the load. This safety check continues to exist. The issue is there in the original code, true. Would the right fix be to add some sanity checks in module_init()? regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS
On Thu, May 16, 2013 at 02:01:12PM +, KY Srinivasan wrote: Would the right fix be to add some sanity checks in module_init()? The check is already there (as I noted above). Look at the function: storvsc_drv_init(). If the ring size is picked incorrectly, the load is failed. Ah. I see that now. My mistake. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] fnic: potential dead lock in fnic_is_abts_pending()
There is an unlock missing if the == FNIC_IOREQ_ABTS_PENDING is
false.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Static analysis. I can't test this.
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index be99e75..a97e6e5 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -2432,11 +2432,9 @@ int fnic_is_abts_pending(struct fnic *fnic, struct
scsi_cmnd *lr_sc)
Found IO in %s on lun\n,
fnic_ioreq_state_to_str(CMD_STATE(sc)));
- if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING) {
- spin_unlock_irqrestore(io_lock, flags);
+ if (CMD_STATE(sc) == FNIC_IOREQ_ABTS_PENDING)
ret = 1;
- continue;
- }
+ spin_unlock_irqrestore(io_lock, flags);
}
return ret;
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
potential buffer overrun in __iscsi_conn_send_pdu()
My static checker complains about a possible array overflow in
__iscsi_conn_send_pdu().
drivers/scsi/libiscsi.c
743 if (data_size) {
744 memcpy(task-data, data, data_size);
745 task-data_count = data_size;
746 } else
747 task-data_count = 0;
748
data_size comes from skb-data and we haven't checked that it is less
than ISCSI_DEF_MAX_RECV_SEG_LEN (8192).
The call tree is:
iscsi_if_recv_msg()
iscsi_conn_send_pdu()
__iscsi_conn_send_pdu()
I'm a newbie to this code, so I'm not sure if this is a real bug or not.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] iscsi-target: missing kfree() on error path
We need to free payload before returning.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/target/iscsi/iscsi_target.c
b/drivers/target/iscsi/iscsi_target.c
index c1106bb..1e59630 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3426,6 +3426,7 @@ static int iscsit_build_sendtargets_response(struct
iscsi_cmd *cmd)
if (!text_ptr) {
pr_err(Unable to locate '=' string in text_in:
%s\n, text_in);
+ kfree(payload);
return -EINVAL;
}
/*
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] target/pscsi: remove an unneeded check
blk_get_request() just returns NULL on error, it doesn't return an
ERR_PTR.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/target/target_core_pscsi.c
b/drivers/target/target_core_pscsi.c
index e992b27..9170b36 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -1050,9 +1050,8 @@ pscsi_execute_cmd(struct se_cmd *cmd)
req = blk_get_request(pdv-pdv_sd-request_queue,
(data_direction == DMA_TO_DEVICE),
GFP_KERNEL);
- if (!req || IS_ERR(req)) {
- pr_err(PSCSI: blk_get_request() failed: %ld\n,
- req ? IS_ERR(req) : -ENOMEM);
+ if (!req) {
+ pr_err(PSCSI: blk_get_request() failed\n);
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
goto fail;
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] mptfusion: zero kernel-space source of copy_to_user
On Wed, Jun 04, 2014 at 12:49:46PM -0400, Joe Lawrence wrote: Fixes the following smatch warning: drivers/message/fusion/mptctl.c:1369 mptctl_getiocinfo() warn: possible info leak 'karg' This is a false positive. I will push a fix for this in Smatch next week. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: scsi_debug: support scsi-mq, queues and locks
[ This is not really a new bug, it's just that renaming the function
made it show up as a new bug and I figured maybe you know what's going
on since you are working with related code. -dan ]
Hello Douglas Gilbert,
This is a semi-automatic email about new static checker warnings.
The patch cbf67842c3d9: scsi_debug: support scsi-mq, queues and
locks from Jul 26, 2014, leads to the following Smatch complaint:
drivers/scsi/scsi_debug.c:4153 scsi_debug_queuecommand()
error: we previously assumed 'cmd' could be null (see line 4106)
drivers/scsi/scsi_debug.c
4105 if ((SCSI_DEBUG_OPT_NOISE scsi_debug_opts)
4106 !(SCSI_DEBUG_OPT_NO_CDB_NOISE scsi_debug_opts) cmd) {
^^^
Check.
4107 char b[120];
4108 int n;
4109
4110 len = SCpnt-cmd_len;
4111 if (len 32)
4112 strcpy(b, too long, over 32 bytes);
4113 else {
4114 for (k = 0, n = 0; k len; ++k)
4115 n += scnprintf(b + n, sizeof(b) - n,
%02x ,
4116 (unsigned int)cmd[k]);
4117 }
4118 sdev_printk(KERN_INFO, SCpnt-device, %s: cmd %s\n,
my_name,
4119 b);
4120 }
4121
4122 if ((SCpnt-device-lun = scsi_debug_max_luns)
4123 (SCpnt-device-lun != SAM2_WLUN_REPORT_LUNS))
4124 return schedule_resp(SCpnt, NULL, DID_NO_CONNECT 16,
0);
4125 devip = devInfoReg(SCpnt-device);
4126 if (NULL == devip)
4127 return schedule_resp(SCpnt, NULL, DID_NO_CONNECT 16,
0);
4128
4129 if ((scsi_debug_every_nth != 0)
4130 (atomic_inc_return(sdebug_cmnd_count) =
4131 abs(scsi_debug_every_nth))) {
4132 atomic_set(sdebug_cmnd_count, 0);
4133 if (scsi_debug_every_nth -1)
4134 scsi_debug_every_nth = -1;
4135 if (SCSI_DEBUG_OPT_TIMEOUT scsi_debug_opts)
4136 return 0; /* ignore command causing timeout */
4137 else if (SCSI_DEBUG_OPT_MAC_TIMEOUT scsi_debug_opts
4138 scsi_medium_access_command(SCpnt))
4139 return 0; /* time out reads and writes */
4140 else if (SCSI_DEBUG_OPT_RECOVERED_ERR scsi_debug_opts)
4141 inj_recovered = 1; /* to reads and writes below
*/
4142 else if (SCSI_DEBUG_OPT_TRANSPORT_ERR scsi_debug_opts)
4143 inj_transport = 1; /* to reads and writes below
*/
4144 else if (SCSI_DEBUG_OPT_DIF_ERR scsi_debug_opts)
4145 inj_dif = 1; /* to reads and writes below */
4146 else if (SCSI_DEBUG_OPT_DIX_ERR scsi_debug_opts)
4147 inj_dix = 1; /* to reads and writes below */
4148 else if (SCSI_DEBUG_OPT_SHORT_TRANSFER
scsi_debug_opts)
4149 inj_short = 1;
4150 }
4151
4152 if (devip-wlun) {
4153 switch (*cmd) {
Unchecked dereference.
4154 case INQUIRY:
4155 case REQUEST_SENSE:
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1.3 2/18] arcmsr: Add code to support MSI-X, MSI interrupt
On Fri, Aug 01, 2014 at 06:38:48PM +0800, Ching Huang wrote: Hi Alexander, Thanks for your advice. This patch was revised according to your comment. Signed-off-by: Chingching2...@areca.com.tw This patch is something that can't be applied at all. There is no changelog. Apply the patch with `cat email.txt | git am` and review the changelog using the `git log` command. https://www.google.com/search?q=how+to+send+a+v2+patch Also the Signed-off-by line is wrong. It should have your full name. There needs to be a space between the name and the email address. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/17] arcmsr: Add code to support system hibernation
On Mon, Aug 11, 2014 at 07:09:55PM +0800, Ching Huang wrote: Yes. 18/18 is obsolete. Thanks to Tomas's advice. There is no way to keep track of all the patches because they aren't in an email thread. Could you resend everything using git-format-patch to send them as one thread? regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: potential buffer overrun in __iscsi_conn_send_pdu()
I never heard back on this. It still looks like a very serious bug
with security implications etc.
regards,
dan carpenter
On Mon, Jun 24, 2013 at 06:46:31PM +0300, Dan Carpenter wrote:
My static checker complains about a possible array overflow in
__iscsi_conn_send_pdu().
drivers/scsi/libiscsi.c
743 if (data_size) {
744 memcpy(task-data, data, data_size);
745 task-data_count = data_size;
746 } else
747 task-data_count = 0;
748
data_size comes from skb-data and we haven't checked that it is less
than ISCSI_DEF_MAX_RECV_SEG_LEN (8192).
The call tree is:
iscsi_if_recv_msg()
iscsi_conn_send_pdu()
__iscsi_conn_send_pdu()
I'm a newbie to this code, so I'm not sure if this is a real bug or not.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] xen-scsifront: use GFP_ATOMIC under spin_lock
This function is only called with a spin_lock held and IRQs disabled. The allocation is not allowed to sleep and NOIO is not sufficient, it has to be ATOMIC. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index 0aceb70..7e88659 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -359,7 +359,7 @@ static int map_data_for_request(struct vscsifrnt_info *info, } seg_grants = vscsiif_grants_sg(data_grants); shadow-sg = kcalloc(data_grants, - sizeof(struct scsiif_request_segment), GFP_NOIO); + sizeof(struct scsiif_request_segment), GFP_ATOMIC); if (!shadow-sg) return -ENOMEM; } -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] xen-scsiback: clean up a type issue in scsiback_make_tpg()
This code was confusing because we had an unsigned long and then we compared it to UINT_MAX and then we stored it in a u16. How many bytes is this supposed to have: 2, 4 or 16??? I've made it a u16 throughout. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index 7b565632..ad11258 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -1885,13 +1885,14 @@ scsiback_make_tpg(struct se_wwn *wwn, struct scsiback_tport, tport_wwn); struct scsiback_tpg *tpg; - unsigned long tpgt; + u16 tpgt; int ret; if (strstr(name, tpgt_) != name) return ERR_PTR(-EINVAL); - if (kstrtoul(name + 5, 10, tpgt) || tpgt UINT_MAX) - return ERR_PTR(-EINVAL); + ret = kstrtou16(name + 5, 10, tpgt); + if (ret) + return ERR_PTR(ret); tpg = kzalloc(sizeof(struct scsiback_tpg), GFP_KERNEL); if (!tpg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
Hello Bradley Grove,
The patch 26780d9e12ed: [SCSI] esas2r: ATTO Technology ExpressSAS 6G
SAS/SATA RAID Adapter Driver from Aug 23, 2013, leads to the
following static checker warning:
drivers/scsi/esas2r/esas2r_ioctl.c:1902 esas2r_read_vda()
error: 'count' from user is not capped properly
drivers/scsi/esas2r/esas2r_ioctl.c
1892
1893 if (off VDA_MAX_BUFFER_SIZE)
1894 return 0;
1895
1896 if (count + off VDA_MAX_BUFFER_SIZE)
^
count is a user controlled int. Let's assume we're on a 32 system for
simplicity. If count is a high positive number here, then count + off
is negative and thus less than VDA_MAX_BUFFER_SIZE.
1897 count = VDA_MAX_BUFFER_SIZE - off;
1898
1899 if (count 0)
1900 return 0;
1901
1902 memcpy(buf, a-vda_buffer + off, count);
^^^
Memory corruption.
1903
1904 return count;
1905 }
count comes from the ioctl. Let's look at that:
drivers/scsi/esas2r/esas2r_ioctl.c
1476 case EXPRESS_IOCTL_VDA:
1477 err = esas2r_write_vda(a,
1478 (char *)ioctl-data.ioctl_vda,
1479 0,
1480 sizeof(struct atto_ioctl_vda) +
1481
ioctl-data.ioctl_vda.data_length);
^^
1482
1483 if (err = 0) {
1484 err = esas2r_read_vda(a,
1485(char
*)ioctl-data.ioctl_vda,
14860,
1487sizeof(struct
atto_ioctl_vda) +
1488
ioctl-data.ioctl_vda.data_length);
^
These additions have integer overflow bugs. It seems harmless to me,
but hopefully static checkers will eventually start complaining about
them.
1489 }
1490
1491
1492
1493
1494 break;
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: qla2xxx: Add FDMI-2 functionality.
Hello Himanshu Madhani,
The patch df57cabac41f: qla2xxx: Add FDMI-2 functionality. from Sep
25, 2014, leads to the following static checker warnings:
drivers/scsi/qla2xxx/qla_gs.c:1339 qla2x00_fdmi_rhba() error: snprintf() chops
off the last chars of 'ha-model_number': 17 vs 16
drivers/scsi/qla2xxx/qla_gs.c:1628 qla2x00_fdmi_rpa() error: snprintf() chops
off the last chars of 'p_sysid-nodename': 65 vs 32
drivers/scsi/qla2xxx/qla_gs.c:1631 qla2x00_fdmi_rpa() error: snprintf() chops
off the last chars of '(((vha-host)-shost_data)-system_hostname)': 256 vs 32
drivers/scsi/qla2xxx/qla_gs.c:1763 qla2x00_fdmiv2_rhba() error: snprintf()
chops off the last chars of 'ha-model_number': 17 vs 16
drivers/scsi/qla2xxx/qla_gs.c:2195 qla2x00_fdmiv2_rpa() error: snprintf() chops
off the last chars of 'p_sysid-nodename': 65 vs 32
drivers/scsi/qla2xxx/qla_gs.c:2198 qla2x00_fdmiv2_rpa() error: snprintf() chops
off the last chars of '(((vha-host)-shost_data)-system_hostname)': 256 vs 32
drivers/scsi/qla2xxx/qla_gs.c
2194 if (p_sysid) {
2195 snprintf(eiter-a.host_name, sizeof(eiter-a.host_name),
2196 %s, p_sysid-nodename);
We're chopping most of the hostname off. That seems bad.
2197 } else {
2198 snprintf(eiter-a.host_name, sizeof(eiter-a.host_name),
2199 %s, fc_host_system_hostname(vha-host));
2200 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: ufs: Add freq-table-hz property for UFS device
Hello Sahitya Tummala,
The patch 4cff6d991e4a: ufs: Add freq-table-hz property for UFS
device from Sep 25, 2014, leads to the following static checker
warning:
drivers/scsi/ufs/ufshcd-pltfrm.c:138 ufshcd_parse_clock_info()
warn: passing devm_ allocated variable to kfree. 'clkfreq'
drivers/scsi/ufs/ufshcd-pltfrm.c
102 clkfreq = devm_kzalloc(dev, sz * sizeof(*clkfreq),
103 GFP_KERNEL);
^
104 if (!clkfreq) {
105 dev_err(dev, %s: no memory\n, freq-table-hz);
Don't print an error. It just wastes ram. Error messages are often
buggy. If we delete it then the code is shorter and easier to read.
Kmalloc() has its own more useful error message already.
106 ret = -ENOMEM;
107 goto out;
Out labels are the worst. The name is too vague. They are a sign of
either One Err type error handling where you have one label handle
everything or they are a just a do-nothing goto where it returns
directly.
One Err type error handling causes bugs.
Do-nothing gotos are just annoying. You're reading the code and you see
a goto out; and you think What does out do? But the name doesn't
provide any hints. You scroll down to the bottom of the function. Oh
it was just a waste of time. Now you have lost your place and your
train of thought.
Also people constantly forget to set ret before the goto out. If you
just write:
if (!of_get_property(np, freq-table-hz, len)) {
dev_info(dev, freq-table-hz property not specified\n);
return 0;
}
if (len = 0)
return 0;
Then it's totally clear what we intended to return. But in the current
code it's ambiguous because maybe we just forgot to set ret? Who
knows. Out labels make the code hard to understand.
Anyway, in this case, originally out was doing One Err error handling
but now it's just there to waste our time and cause bugs when this code
is modified in the future.
108 }
109
110 ret = of_property_read_u32_array(np, freq-table-hz,
111 clkfreq, sz);
112 if (ret (ret != -EINVAL)) {
113 dev_err(dev, %s: error reading array %d\n,
114 freq-table-hz, ret);
115 goto free_clkfreq;
116 }
117
118 for (i = 0; i sz; i += 2) {
119 ret = of_property_read_string_index(np,
120 clock-names, i/2, (const char
**)name);
121 if (ret)
122 goto free_clkfreq;
123
124 clki = devm_kzalloc(dev, sizeof(*clki), GFP_KERNEL);
125 if (!clki) {
126 ret = -ENOMEM;
127 goto free_clkfreq;
128 }
129
130 clki-min_freq = clkfreq[i];
131 clki-max_freq = clkfreq[i+1];
132 clki-name = kstrdup(name, GFP_KERNEL);
Where is name freed? There are definitely certain error paths where it
is leaked. Can we use devm_kstrdup() here?
133 dev_dbg(dev, %s: min %u max %u name %s\n,
freq-table-hz,
134 clki-min_freq, clki-max_freq,
clki-name);
135 list_add_tail(clki-list, hba-clk_list_head);
136 }
137 free_clkfreq:
138 kfree(clkfreq);
^
Just delete this.
139 out:
140 return ret;
141 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: ufs: Add regulator enable support
Hello Sujit Reddy Thumma,
The patch aa4976130934: ufs: Add regulator enable support from Sep
25, 2014, leads to the following static checker warning:
drivers/scsi/ufs/ufshcd-pltfrm.c:167 ufshcd_populate_vreg()
warn: missing error code here? 'devm_kzalloc()' failed. 'ret' = '0'
drivers/scsi/ufs/ufshcd-pltfrm.c
144 static int ufshcd_populate_vreg(struct device *dev, const char *name,
145 struct ufs_vreg **out_vreg)
146 {
147 int ret = 0;
148 char prop_name[MAX_PROP_SIZE];
149 struct ufs_vreg *vreg = NULL;
150 struct device_node *np = dev-of_node;
151
152 if (!np) {
153 dev_err(dev, %s: non DT initialization\n, __func__);
154 goto out;
155 }
156
157 snprintf(prop_name, MAX_PROP_SIZE, %s-supply, name);
158 if (!of_parse_phandle(np, prop_name, 0)) {
159 dev_info(dev, %s: Unable to find %s regulator,
assuming enabled\n,
160 __func__, prop_name);
161 goto out;
162 }
163
164 vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
165 if (!vreg) {
166 dev_err(dev, No memory for %s regulator\n, name);
167 goto out;
Don't print an error message.
Don't use a goto out.
Don't forget to set an error code.
168 }
169
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: ufs: add UFS power management support
Hello Subhash Jadavani,
The patch 57d104c153d3: ufs: add UFS power management support from
Sep 25, 2014, leads to the following static checker warning:
drivers/scsi/ufs/ufshcd.c:4746 ufshcd_link_state_transition()
warn: we tested 'check_for_bkops' before and it was 'true'
drivers/scsi/ufs/ufshcd.c
4734 if (req_link_state == UIC_LINK_HIBERN8_STATE) {
4735 ret = ufshcd_uic_hibern8_enter(hba);
4736 if (!ret)
4737 ufshcd_set_link_hibern8(hba);
4738 else
4739 goto out;
4740 }
4741 /*
4742 * If autobkops is enabled, link can't be turned off because
4743 * turning off the link would also turn off the device.
4744 */
4745 else if ((req_link_state == UIC_LINK_OFF_STATE)
4746 (!check_for_bkops || (check_for_bkops
^^^
Not needed.
4747 !hba-auto_bkops_enabled))) {
4748 /*
4749 * Change controller state to reset state which
4750 * should also put the link in off/reset state
4751 */
4752 ufshcd_hba_stop(hba);
4753 /*
4754 * TODO: Check if we need any delay to make sure that
4755 * controller is reset
4756 */
4757 ufshcd_set_link_off(hba);
4758 }
4759
4760 out:
4761 return ret;
4762 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: ufs: Add support for clock gating
Hello Sahitya Tummala,
The patch 1ab27c9cf8b6: ufs: Add support for clock gating from Sep
25, 2014, leads to the following static checker warning:
drivers/scsi/ufs/ufshcd.c:4474 __ufshcd_setup_clocks()
warn: we tested 'ret' before and it was 'false'
drivers/scsi/ufs/ufshcd.c
4467 ret = hba-vops-setup_clocks(hba, on);
4468 out:
4469 if (ret) {
4470 list_for_each_entry(clki, head, list) {
4471 if (!IS_ERR_OR_NULL(clki-clk) clki-enabled)
4472 clk_disable_unprepare(clki-clk);
4473 }
4474 } else if (!ret on) {
Not needed.
4475 spin_lock_irqsave(hba-host-host_lock, flags);
4476 hba-clk_gating.state = CLKS_ON;
4477 spin_unlock_irqrestore(hba-host-host_lock, flags);
4478 }
4479 return ret;
4480 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: ufs: definitions for phy interface
Hello Dolev Raviv, This is a semi-automatic email about new static checker warnings. The patch e785060ea3a1: ufs: definitions for phy interface from Sep 25, 2014, leads to the following Smatch complaint: drivers/scsi/ufs/ufshcd.c:5118 ufshcd_system_suspend() error: we previously assumed 'hba' could be null (see line 5089) drivers/scsi/ufs/ufshcd.c 5088 5089 if (!hba || !hba-is_powered) Existing check for NULL. 5090 goto out; 5091 [ snip ] 5115 ret = ufshcd_suspend(hba, UFS_SYSTEM_PM); 5116 out: 5117 if (!ret) 5118 hba-is_sys_suspended = true; ^ New unchecked dereference. This is a One Err Bug caused by out label style error handling. 5119 return ret; 5120 } regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] scsi: bfa: bfa_fcs_lport.c: Cleaning up missing null-terminate by switching from strncpy to strzcpy
On Sun, Oct 19, 2014 at 12:13:10AM +0200, Rickard Strandqvist wrote: Ensures that the string is null-terminate in connection with the use of strncpy, by switching from strncpy to strzcpy. I wish the changelogs were more clear that we have no reason to think this is a real issue. Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se --- drivers/scsi/bfa/bfa_fcs_lport.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c index ff75ef8..78af89e 100644 --- a/drivers/scsi/bfa/bfa_fcs_lport.c +++ b/drivers/scsi/bfa/bfa_fcs_lport.c @@ -2633,7 +2633,7 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi, strncpy(hba_attr-driver_version, (char *)driver_info-version, sizeof(hba_attr-driver_version)); Why do we not change this one? - strncpy(hba_attr-os_name, driver_info-host_os_name, + strzcpy(hba_attr-os_name, driver_info-host_os_name, sizeof(hba_attr-os_name)); strlcpy() would also be totally safe here. It's also safe for patch patch 3 and patch 4. Patch 5 is trickier to audit so I don't know if strlcpy() is safe. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ufs: add UFS power management support
On Thu, Oct 23, 2014 at 10:28:52AM +0300, Dolev Raviv wrote:
Hi Dan,
This seem like a false alarm, please let me know if it requires a fix.
It may not indicate a bug, but it is correct in that the code could be
re-written:
Old:
else if ((req_link_state == UIC_LINK_OFF_STATE)
(!check_for_bkops || (check_for_bkops
!hba-auto_bkops_enabled))) {
Proposed:
else if ((req_link_state == UIC_LINK_OFF_STATE)
(!check_for_bkops || !hba-auto_bkops_enabled)) {
Those two are the same.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/2] bnx2fc: fix an error code in _bnx2fc_create()
We should be returning an error code here instead of success. Either
-ENODEV or -ENOMEM would work. There is also a failure message in
printk().
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index dfed4be..251b114 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -2195,6 +2195,7 @@ static int _bnx2fc_create(struct net_device *netdev,
interface = bnx2fc_interface_create(hba, netdev, fip_mode);
if (!interface) {
printk(KERN_ERR PFX bnx2fc_interface_create failed\n);
+ rc = -ENOMEM;
goto ifput_err;
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] scsi: missing braces in scsi_extd_sense_format()
There were missing curly braces so we always return the first
additional2[] string.
Fixes: 7046d2fa6dbd ('scsi: use sdev as argument for sense code printing')
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index a1a7fca..0cf43f6 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1282,9 +1282,10 @@ scsi_extd_sense_format(unsigned char asc, unsigned char
ascq, const char **fmt)
for (i = 0; additional2[i].fmt; i++) {
if (additional2[i].code1 == asc
ascq = additional2[i].code2_min
- ascq = additional2[i].code2_max)
+ ascq = additional2[i].code2_max) {
*fmt = additional2[i].fmt;
return additional2[i].str;
+ }
}
#endif
return NULL;
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] usb: storage: debug: uninitialized var in usb_stor_show_sense()
The fmt variable might be used uninitialized, it should be set to NULL
at the start.
Fixes: d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/usb/storage/debug.c b/drivers/usb/storage/debug.c
index 57bf3ad..fd00509 100644
--- a/drivers/usb/storage/debug.c
+++ b/drivers/usb/storage/debug.c
@@ -164,7 +164,8 @@ void usb_stor_show_sense(const struct us_data *us,
unsigned char asc,
unsigned char ascq)
{
- const char *what, *keystr, *fmt;
+ const char *fmt = NULL;
+ const char *what, *keystr;
keystr = scsi_sense_key_string(key);
what = scsi_extd_sense_format(asc, ascq, fmt);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] usb: storage: debug: uninitialized var in usb_stor_show_sense()
On Sat, Nov 29, 2014 at 08:07:55AM -0800, James Bottomley wrote:
On Sat, 2014-11-29 at 15:48 +0300, Dan Carpenter wrote:
The fmt variable might be used uninitialized, it should be set to NULL
at the start.
Fixes: d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
Hm, that' falls into the category of an interface that's hard to get
right if every caller has to remember to set fmt to NULL.
Could you instead set *fmt to NULL in scsi_extd_sense_format()? That
way the interface is much easier.
Sure. I will send that tomorrow.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] scsi: set fmt to NULL scsi_extd_sense_format() by default
One of the two callers passes an unintialized pointer as fmt and
expects it to be set to NULL if there is no format string. Let's make
this function work as expected.
Fixes: d811b848ebb7 ('scsi: use sdev as argument for sense code printing')
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
v2: Change scsi_extd_sense_format() instead of the caller
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index aee62f6..e2068a2 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1271,6 +1271,7 @@ scsi_extd_sense_format(unsigned char asc, unsigned char
ascq, const char **fmt)
int i;
unsigned short code = ((asc 8) | ascq);
+ *fmt = NULL;
for (i = 0; additional[i].text; i++)
if (additional[i].code12 == code)
return additional[i].text;
@@ -1282,6 +1283,8 @@ scsi_extd_sense_format(unsigned char asc, unsigned char
ascq, const char **fmt)
return additional2[i].str;
}
}
+#else
+ *fmt = NULL;
#endif
return NULL;
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] mpt2sas: issue reset is not uninitialized
The issue_reset variables were never set to zero. This bug was found with static analysis. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c index 58e4521..98bd546 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c @@ -3236,7 +3236,7 @@ mpt2sas_base_sas_iounit_control(struct MPT2SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = 0; int rc; void *request; u16 wait_state_count; @@ -3341,7 +3341,7 @@ mpt2sas_base_scsi_enclosure_processor(struct MPT2SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = 0; int rc; void *request; u16 wait_state_count; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] mpt3sas: issue_reset is uninitialized
We never set issue_reset to zero. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 1560115..a6d471a 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -3419,7 +3419,7 @@ mpt3sas_base_sas_iounit_control(struct MPT3SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = 0; int rc; void *request; u16 wait_state_count; @@ -3523,7 +3523,7 @@ mpt3sas_base_scsi_enclosure_processor(struct MPT3SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = 0; int rc; void *request; u16 wait_state_count; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] mpt2sas: issue reset is not uninitialized
On Wed, Dec 03, 2014 at 11:36:20AM +0100, Julia Lawall wrote: On Wed, 3 Dec 2014, Dan Carpenter wrote: The issue_reset variables were never set to zero. This bug was found with static analysis. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c index 58e4521..98bd546 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c @@ -3236,7 +3236,7 @@ mpt2sas_base_sas_iounit_control(struct MPT2SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = 0; Why not false if it is going to be a boolean? Good point. I'll send a v2 for these tomorrow. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] mpt2sas: issue reset is not uninitialized
On Wed, Dec 03, 2014 at 03:14:08PM +0100, walter harms wrote: hi Dan, just a minor question: did you forget to mention that you also changed from u8 to bool or was this unintentional ? That's a trivial thing and I didn't think it was worth mentioning. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] mpt2sas: issue reset is never uninitialized
The issue_reset variable can be used uninitialized. It should be set to false at the start. Also I cleaned up the types by using bool. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- v2: style fix diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c index 58e4521..3fb01d1 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c @@ -3236,7 +3236,7 @@ mpt2sas_base_sas_iounit_control(struct MPT2SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = false; int rc; void *request; u16 wait_state_count; @@ -3300,7 +3300,7 @@ mpt2sas_base_sas_iounit_control(struct MPT2SAS_ADAPTER *ioc, _debug_dump_mf(mpi_request, sizeof(Mpi2SasIoUnitControlRequest_t)/4); if (!(ioc-base_cmds.status MPT2_CMD_RESET)) - issue_reset = 1; + issue_reset = true; goto issue_host_reset; } if (ioc-base_cmds.status MPT2_CMD_REPLY_VALID) @@ -3341,7 +3341,7 @@ mpt2sas_base_scsi_enclosure_processor(struct MPT2SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = false; int rc; void *request; u16 wait_state_count; @@ -3398,7 +3398,7 @@ mpt2sas_base_scsi_enclosure_processor(struct MPT2SAS_ADAPTER *ioc, _debug_dump_mf(mpi_request, sizeof(Mpi2SepRequest_t)/4); if (!(ioc-base_cmds.status MPT2_CMD_RESET)) - issue_reset = 1; + issue_reset = true; goto issue_host_reset; } if (ioc-base_cmds.status MPT2_CMD_REPLY_VALID) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] mpt3sas: issue_reset is uninitialized
The issue_reset can be used uninitialized. It should be set to false at the start. Also I cleaned up the types a little by using bool. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- v2: style fix. use true/false instead of 1/0. diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 1560115..b9c2739 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -3419,7 +3419,7 @@ mpt3sas_base_sas_iounit_control(struct MPT3SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = false; int rc; void *request; u16 wait_state_count; @@ -3483,7 +3483,7 @@ mpt3sas_base_sas_iounit_control(struct MPT3SAS_ADAPTER *ioc, _debug_dump_mf(mpi_request, sizeof(Mpi2SasIoUnitControlRequest_t)/4); if (!(ioc-base_cmds.status MPT3_CMD_RESET)) - issue_reset = 1; + issue_reset = true; goto issue_host_reset; } if (ioc-base_cmds.status MPT3_CMD_REPLY_VALID) @@ -3523,7 +3523,7 @@ mpt3sas_base_scsi_enclosure_processor(struct MPT3SAS_ADAPTER *ioc, u16 smid; u32 ioc_state; unsigned long timeleft; - u8 issue_reset; + bool issue_reset = false; int rc; void *request; u16 wait_state_count; @@ -3581,7 +3581,7 @@ mpt3sas_base_scsi_enclosure_processor(struct MPT3SAS_ADAPTER *ioc, _debug_dump_mf(mpi_request, sizeof(Mpi2SepRequest_t)/4); if (!(ioc-base_cmds.status MPT3_CMD_RESET)) - issue_reset = 1; + issue_reset = false; goto issue_host_reset; } if (ioc-base_cmds.status MPT3_CMD_REPLY_VALID) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [bug report] [SCSI] mvumi: GFP_KERNEL under spin lock
Whatever happened with this? You could just change the GFP_KERNEL to GFP_ATOMIC. regards, dan carpenter On Tue, Aug 14, 2012 at 05:59:27PM +0300, Dan Carpenter wrote: Hello Jianyun Li, The patch f0c568a478f0: [SCSI] mvumi: Add Marvell UMI driver from May 11, 2011, leads to the following warning: drivers/scsi/mvumi.c:121 mvumi_alloc_mem_resource() error: scheduling with locks held: 'spin_lock:host_lock' The problem is that we do a couple GPF_KERNEL allocations in mvumi_alloc_mem_resource() and this static analysis program sees a path where that function is called with spin_locks held. mvumi_isr_handler() - takes a spin lock - mvumi_handshake() - mvumi_init_data() - mvumi_alloc_mem_resource() - GFP_KERNEL The IRQ handler does print a warning before calling mvumi_handshake() so it seems like this path doesn't get exercised very much. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [bug report] [SCSI] mvumi: GFP_KERNEL under spin lock
On Fri, Sep 28, 2012 at 02:30:22PM +0300, Dan Carpenter wrote: Whatever happened with this? You could just change the GFP_KERNEL to GFP_ATOMIC. Ah, that email address is dead is what happened. I'll send a patch that changes this to GFP_ATOMIC. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] mvumi: use GFP_ATOMIC under spin lock
This is called from the interrupt handler and with spin_locks held. Use
GFP_ATOMIC. The call tree looks like:
mvumi_isr_handler() - takes a spin lock
- mvumi_handshake()
- mvumi_init_data()
- mvumi_alloc_mem_resource() - GFP_KERNEL
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
index 783edc7..5ad8da4 100644
--- a/drivers/scsi/mvumi.c
+++ b/drivers/scsi/mvumi.c
@@ -118,7 +118,7 @@ static int mvumi_map_pci_addr(struct pci_dev *dev, void
**addr_array)
static struct mvumi_res *mvumi_alloc_mem_resource(struct mvumi_hba *mhba,
enum resource_type type, unsigned int size)
{
- struct mvumi_res *res = kzalloc(sizeof(*res), GFP_KERNEL);
+ struct mvumi_res *res = kzalloc(sizeof(*res), GFP_ATOMIC);
if (!res) {
dev_err(mhba-pdev-dev,
@@ -128,7 +128,7 @@ static struct mvumi_res *mvumi_alloc_mem_resource(struct
mvumi_hba *mhba,
switch (type) {
case RESOURCE_CACHED_MEMORY:
- res-virt_addr = kzalloc(size, GFP_KERNEL);
+ res-virt_addr = kzalloc(size, GFP_ATOMIC);
if (!res-virt_addr) {
dev_err(mhba-pdev-dev,
unable to allocate memory,size = %d.\n, size);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] target/iscsi: precedence bug in iscsit_set_dataout_sequence_values()
Clang warns about this bug:
drivers/target/iscsi/iscsi_target_erl0.c:52:45: warning: operator '?:'
has lower precedence than '+'; '+' will be evaluated first
[-Wparentheses]
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Please review this very carefully because I haven't tested it. It could
be that the check should be:
(data_done + data_length FirstBurstLength) ? FirstBurstLength : data_length);
Instead of what I have which is:
data_done + (data_length FirstBurstLength ? FirstBurstLength : data_length);
diff --git a/drivers/target/iscsi/iscsi_target_erl0.c
b/drivers/target/iscsi/iscsi_target_erl0.c
index 1a02016..2067efd 100644
--- a/drivers/target/iscsi/iscsi_target_erl0.c
+++ b/drivers/target/iscsi/iscsi_target_erl0.c
@@ -48,9 +48,9 @@ void iscsit_set_dataout_sequence_values(
if (cmd-unsolicited_data) {
cmd-seq_start_offset = cmd-write_data_done;
cmd-seq_end_offset = (cmd-write_data_done +
- (cmd-se_cmd.data_length
-conn-sess-sess_ops-FirstBurstLength) ?
- conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length);
+ ((cmd-se_cmd.data_length
+ conn-sess-sess_ops-FirstBurstLength) ?
+conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length));
return;
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] target/iscsi: precedence bug in iscsit_set_dataout_sequence_values()
On Tue, Oct 02, 2012 at 01:48:57PM +0200, walter harms wrote:
Am 02.10.2012 10:22, schrieb Dan Carpenter:
Clang warns about this bug:
drivers/target/iscsi/iscsi_target_erl0.c:52:45: warning: operator '?:'
has lower precedence than '+'; '+' will be evaluated first
[-Wparentheses]
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Please review this very carefully because I haven't tested it. It could
be that the check should be:
(data_done + data_length FirstBurstLength) ? FirstBurstLength :
data_length);
Instead of what I have which is:
data_done + (data_length FirstBurstLength ? FirstBurstLength :
data_length);
diff --git a/drivers/target/iscsi/iscsi_target_erl0.c
b/drivers/target/iscsi/iscsi_target_erl0.c
index 1a02016..2067efd 100644
--- a/drivers/target/iscsi/iscsi_target_erl0.c
+++ b/drivers/target/iscsi/iscsi_target_erl0.c
@@ -48,9 +48,9 @@ void iscsit_set_dataout_sequence_values(
if (cmd-unsolicited_data) {
cmd-seq_start_offset = cmd-write_data_done;
cmd-seq_end_offset = (cmd-write_data_done +
- (cmd-se_cmd.data_length
-conn-sess-sess_ops-FirstBurstLength) ?
- conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length);
+ ((cmd-se_cmd.data_length
+ conn-sess-sess_ops-FirstBurstLength) ?
+conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length));
return;
}
the ?: operator is nice but at a certain length is starts to become
unreadable,
the end is normally calculated by end= start+len; Therefor i suggest:
if ( cmd-se_cmd.data_length conn-sess-sess_ops-FirstBurstLength )
cmd-seq_end_offset = cmd-seq_start_offset +
conn-sess-sess_ops-FirstBurstLength;
else
cmd-seq_end_offset = cmd-seq_start_offset + cmd-se_cmd.data_length;
Yeah. It's not beautiful.
Doing a struct iscsi_sess_ops *ops = conn-sess-sess_ops; at the
start of the function would help as well.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] bfa: unlock on error in bfad_iocmd_cfg_trunk()
We added a new return but forgot to drop the lock first.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Bug introduced in e353546e [SCSI] bfa: Add diagnostic port (D-Port)
support.
diff --git a/drivers/scsi/bfa/bfad_bsg.c b/drivers/scsi/bfa/bfad_bsg.c
index 555e7db..91465b2 100644
--- a/drivers/scsi/bfa/bfad_bsg.c
+++ b/drivers/scsi/bfa/bfad_bsg.c
@@ -2238,8 +2238,10 @@ bfad_iocmd_cfg_trunk(struct bfad_s *bfad, void *cmd,
unsigned int v_cmd)
spin_lock_irqsave(bfad-bfad_lock, flags);
- if (bfa_fcport_is_dport(bfad-bfa))
+ if (bfa_fcport_is_dport(bfad-bfa)) {
+ spin_unlock_irqrestore(bfad-bfad_lock, flags);
return BFA_STATUS_DPORT_ERR;
+ }
if ((fcport-cfg.topology == BFA_PORT_TOPOLOGY_LOOP) ||
(fcport-topology == BFA_PORT_TOPOLOGY_LOOP))
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] bfa: cleanup a memcpy()
Smatch complains that we are writing more data than -srlid_base
member can hold. In fact, we are over writing the whole struct. I've
re-written it to be a bit more clear and to silence the static checker
warning.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index 0116c10..a6dc18e 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -3624,11 +3624,9 @@ bfa_sfp_show_comp(struct bfa_sfp_s *sfp, struct
bfi_mbmsg_s *msg)
bfa_trc(sfp, sfp-memtype);
if (sfp-memtype == BFI_SFP_MEM_DIAGEXT) {
bfa_trc(sfp, sfp-data_valid);
- if (sfp-data_valid) {
- u32 size = sizeof(struct sfp_mem_s);
- u8 *des = (u8 *) (sfp-sfpmem-srlid_base);
- memcpy(des, sfp-dbuf_kva, size);
- }
+ if (sfp-data_valid)
+ memcpy(sfp-sfpmem, sfp-dbuf_kva,
+ sizeof(*sfp-sfpmem));
/*
* Queue completion callback.
*/
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] qla4xxx: fix flash/ddb support
Hello Mike Christie,
The patch 13483730a13b: [SCSI] qla4xxx: fix flash/ddb support from
Dec 1, 2011, leads to the following warning:
drivers/scsi/qla4xxx/ql4_os.c:714 qla4xxx_ep_connect()
error: memcpy() 'dst_addr' too small (16 vs 28)
I've sort of reported this bug before because it exhibits itself in more
than one way.
4684 static struct iscsi_endpoint *qla4xxx_get_ep_fwdb(struct scsi_qla_host
*ha,
4685 struct dev_db_entry
*fw_ddb_entry)
4686 {
4687 struct iscsi_endpoint *ep;
4688 struct sockaddr_in *addr;
4689 struct sockaddr_in6 *addr6;
4690 struct sockaddr *dst_addr;
addr6 is 28 bytes.
dst_addr is 16 bytes.
4691 char *ip;
4692
4693 /* TODO: need to destroy on unload iscsi_endpoint*/
4694 dst_addr = vmalloc(sizeof(*dst_addr));
We allocate 16 bytes.
4695 if (!dst_addr)
4696 return NULL;
4697
4698 if (fw_ddb_entry-options DDB_OPT_IPV6_DEVICE) {
4699 dst_addr-sa_family = AF_INET6;
4700 addr6 = (struct sockaddr_in6 *)dst_addr;
4701 ip = (char *)addr6-sin6_addr;
4702 memcpy(ip, fw_ddb_entry-ip_addr, IPv6_ADDR_LEN);
This memcpy() is copying 16 bytes into (u8 *)dst_addr + 8 so it's
corrupting 8 bytes of data past the end of the dst_addr struct.
4703 addr6-sin6_port =
htons(le16_to_cpu(fw_ddb_entry-port));
4704
4705 } else {
4706 dst_addr-sa_family = AF_INET;
4707 addr = (struct sockaddr_in *)dst_addr;
4708 ip = (char *)addr-sin_addr;
4709 memcpy(ip, fw_ddb_entry-ip_addr, IP_ADDR_LEN);
4710 addr-sin_port = htons(le16_to_cpu(fw_ddb_entry-port));
4711 }
4712
4713 ep = qla4xxx_ep_connect(ha-host, dst_addr, 0);
There is another memcpy() inside the call to qla4xxx_ep_connect() which
reads beyond the end of the array.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] MegaRAID: use GFP_ATOMIC under spin lock
We're holding a spin_lock here so we should use GFP_ATOMIC. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c index 25506c7..5205275 100644 --- a/drivers/scsi/megaraid/megaraid_mm.c +++ b/drivers/scsi/megaraid/megaraid_mm.c @@ -568,7 +568,7 @@ mraid_mm_attach_buf(mraid_mmadp_t *adp, uioc_t *kioc, int xferlen) kioc-pool_index= right_pool; kioc-free_buf = 1; - kioc-buf_vaddr = pci_pool_alloc(pool-handle, GFP_KERNEL, + kioc-buf_vaddr = pci_pool_alloc(pool-handle, GFP_ATOMIC, kioc-buf_paddr); spin_unlock_irqrestore(pool-lock, flags); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] csiostor: remove unneeded memset()
No need to memset() this when we just copy over it on the next line. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/csiostor/csio_lnode.c b/drivers/scsi/csiostor/csio_lnode.c index 551959e..8b54976 100644 --- a/drivers/scsi/csiostor/csio_lnode.c +++ b/drivers/scsi/csiostor/csio_lnode.c @@ -245,7 +245,6 @@ csio_append_attrib(uint8_t **ptr, uint16_t type, uint8_t *val, uint16_t len) len += 4; /* includes attribute type and length */ len = (len + 3) ~3; /* should be multiple of 4 bytes */ ae-len = htons(len); - memset(ae-value, 0, len - 4); memcpy(ae-value, val, len); *ptr += len; } -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] mpt3sas: cut and paste bug storing trigger mpi
ioc-diag_trigger_mpi is an SL_WH_MPI_TRIGGERS_T struct. There is a cut and paste error here and SL_WH_EVENT_TRIGGERS_T is used instead of SL_WH_MPI_TRIGGERS_T. Since the SL_WH_EVENT_TRIGGERS_T is smaller than SL_WH_MPI_TRIGGERS_T, it means we only clear part of the buffer. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Only needed in linux-next. This is a static analysis patch. Even though I'm pretty sure it's correct, I'm not able to test it. diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index 8af944d..3e35e64 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -3136,7 +3136,7 @@ _ctl_diag_trigger_mpi_store(struct device *cdev, spin_lock_irqsave(ioc-diag_trigger_lock, flags); sz = min(sizeof(struct SL_WH_MPI_TRIGGERS_T), count); memset(ioc-diag_trigger_mpi, 0, - sizeof(struct SL_WH_EVENT_TRIGGERS_T)); + sizeof(struct SL_WH_MPI_TRIGGERS_T)); memcpy(ioc-diag_trigger_mpi, buf, sz); if (ioc-diag_trigger_mpi.ValidEntries NUM_VALID_ENTRIES) ioc-diag_trigger_mpi.ValidEntries = NUM_VALID_ENTRIES; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] [SCSI] mpt3sas: cut and paste bug storing trigger mpi
ioc-diag_trigger_mpi is an SL_WH_MPI_TRIGGERS_T struct. There is a cut and paste error here and SL_WH_EVENT_TRIGGERS_T is used instead of SL_WH_MPI_TRIGGERS_T. Since the SL_WH_EVENT_TRIGGERS_T is smaller than SL_WH_MPI_TRIGGERS_T, it means we only clear part of the buffer. I've changed it to use sizeof(ioc-diag_trigger_mpi) which is a bit simpler. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- v2: Style tweak. Only needed in linux-next. This is a static analysis patch. Even though I'm pretty sure it's correct, I'm not able to test it. diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index 8af944d..3e35e64 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -3136,7 +3136,7 @@ _ctl_diag_trigger_mpi_store(struct device *cdev, spin_lock_irqsave(ioc-diag_trigger_lock, flags); sz = min(sizeof(struct SL_WH_MPI_TRIGGERS_T), count); memset(ioc-diag_trigger_mpi, 0, - sizeof(struct SL_WH_EVENT_TRIGGERS_T)); + sizeof(ioc-diag_trigger_mpi)); memcpy(ioc-diag_trigger_mpi, buf, sz); if (ioc-diag_trigger_mpi.ValidEntries NUM_VALID_ENTRIES) ioc-diag_trigger_mpi.ValidEntries = NUM_VALID_ENTRIES; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [SCSI] mpt3sas: cut and paste bug storing trigger mpi
On Fri, Dec 07, 2012 at 09:41:56AM +0100, Rolf Eike Beer wrote: memset(ioc-diag_trigger_mpi, 0, -sizeof(struct SL_WH_EVENT_TRIGGERS_T)); +sizeof(struct SL_WH_MPI_TRIGGERS_T)); memcpy(ioc-diag_trigger_mpi, buf, sz); if (ioc-diag_trigger_mpi.ValidEntries NUM_VALID_ENTRIES) ioc-diag_trigger_mpi.ValidEntries = NUM_VALID_ENTRIES; Then just use sizeof(ioc-diag_trigger_mpi), then it's irrelevant how that type is ever called. Yeah, that's the way I would have prefered to write this as well. It took me a while to verify that the intent here was not to clear only part of the struct. Using sizeof(variable_name) instead of sizeof(type) is more readable. But then you get into the thing where the line before uses sizeof(type)... Oh well, I've sent both styles and the maintainer can choose. :) regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[scsi:for-next 12/27] drivers/scsi/ipr.c:6568 ipr_qc_issue() warn: inconsistent returns spin_lock:ipr_cmd-hrrq-_lock: locked (6562) unlocked (6499,6507,6568)
Hi,
FYI, there are new smatch warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
head: e3ff197a750d2912d0bb2a0161c23c18bad250ad
commit: e7ef7307d9e2c15802e8ea1da9878e0550ec87bc [12/27] [SCSI] ipr: Reduce
lock contention
drivers/scsi/ipr.c:5434 ipr_build_ioadl64() info: why not propagate 'nseg'
from scsi_dma_map() instead of (-1)?
drivers/scsi/ipr.c:5485 ipr_build_ioadl() info: why not propagate 'nseg' from
scsi_dma_map() instead of (-1)?
+ drivers/scsi/ipr.c:6568 ipr_qc_issue() warn: inconsistent returns
spin_lock:ipr_cmd-hrrq-_lock: locked (6562) unlocked (6499,6507,6568)
git remote add scsi git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git
git remote update scsi
git checkout e7ef7307d9e2c15802e8ea1da9878e0550ec87bc
vim +6568 drivers/scsi/ipr.c
35a39691 Brian King 2006-09-25 6493
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6494 if (qc-lldd_task ==
NULL)
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6495
ipr_qc_defer(qc);
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6496
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6497 ipr_cmd = qc-lldd_task;
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6498 if (ipr_cmd == NULL)
0feeed82 Brian King 2007-03-29 @6499 return
AC_ERR_SYSTEM;
35a39691 Brian King 2006-09-25 6500
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6501 qc-lldd_task = NULL;
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6502
spin_lock(ipr_cmd-hrrq-_lock);
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6503 if
(unlikely(!ipr_cmd-hrrq-allow_cmds ||
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6504
ipr_cmd-hrrq-ioa_is_dead)) {
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6505
list_add_tail(ipr_cmd-queue, ipr_cmd-hrrq-hrrq_free_q);
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6506
spin_unlock(ipr_cmd-hrrq-_lock);
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 @6507 return
AC_ERR_SYSTEM;
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6508 }
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6509
7af0de31 wenxi...@linux.vnet.ibm.com 2012-12-03 6510
ipr_init_ipr_cmnd(ipr_cmd, ipr_lock_and_done);
35a39691 Brian King 2006-09-25 6511 ioarcb =
ipr_cmd-ioarcb;
35a39691 Brian King 2006-09-25 6512
a32c055f Wayne Boyer 2010-02-19 6513 if (ioa_cfg-sis64) {
a32c055f Wayne Boyer 2010-02-19 6514 regs =
ipr_cmd-i.ata_ioadl.regs;
a32c055f Wayne Boyer 2010-02-19 6515
ioarcb-add_cmd_parms_offset = cpu_to_be16(sizeof(*ioarcb));
a32c055f Wayne Boyer 2010-02-19 6516 } else
a32c055f Wayne Boyer 2010-02-19 6517 regs =
ioarcb-u.add_data.u.regs;
a32c055f Wayne Boyer 2010-02-19 6518
a32c055f Wayne Boyer 2010-02-19 6519 memset(regs, 0,
sizeof(*regs));
a32c055f Wayne Boyer 2010-02-19 6520
ioarcb-add_cmd_parms_len = cpu_to_be16(sizeof(*regs));
35a39691 Brian King 2006-09-25 6521
e7ef7307 wenxi...@linux.vnet.ibm.com 2012-12-03 6522
list_add_tail(ipr_cmd-queue, ipr_cmd-hrrq-hrrq_pending_q);
35a39691 Brian King 2006-09-25 6523 ipr_cmd-qc = qc;
35a39691 Brian King 2006-09-25 6524 ipr_cmd-done =
ipr_sata_done;
3e7ebdfa Wayne Boyer 2010-02-19 6525
ipr_cmd-ioarcb.res_handle = res-res_handle;
35a39691 Brian King 2006-09-25 6526
ioarcb-cmd_pkt.request_type = IPR_RQTYPE_ATA_PASSTHRU;
35a39691 Brian King 2006-09-25 6527
ioarcb-cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_LINK_DESC;
35a39691 Brian King 2006-09-25 6528
ioarcb-cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_ULEN_CHK;
dde20207 James Bottomley 2008-02-19 6529 ipr_cmd-dma_use_sg =
qc-n_elem;
35a39691 Brian King 2006-09-25 6530
a32c055f Wayne Boyer 2010-02-19 6531 if (ioa_cfg-sis64)
a32c055f Wayne Boyer 2010-02-19 6532
ipr_build_ata_ioadl64(ipr_cmd, qc);
a32c055f Wayne Boyer 2010-02-19 6533 else
a32c055f Wayne Boyer 2010-02-19 6534
ipr_build_ata_ioadl(ipr_cmd, qc);
a32c055f Wayne Boyer 2010-02-19 6535
35a39691 Brian King 2006-09-25 6536 regs-flags |=
IPR_ATA_FLAG_STATUS_ON_GOOD_COMPLETION;
35a39691 Brian King 2006-09-25 6537 ipr_copy_sata_tf(regs,
qc-tf);
35a39691 Brian King 2006-09-25 6538
memcpy(ioarcb-cmd_pkt.cdb, qc-cdb, IPR_MAX_CDB_LEN);
3e7ebdfa Wayne Boyer 2010-02-19 6539 ipr_trc_hook(ipr_cmd,
IPR_TRACE_START, IPR_GET_RES_PHYS_LOC(res));
35a39691 Brian King 2006-09-25 6540
35a39691 Brian King 2006-09-25 6541
[scsi:for-next 23/27] drivers/scsi/fnic/fnic_scsi.c:1260 fnic_rport_exch_reset() error: we previously assumed 'sc-device' could be null (see line 1237)
[ It doesn't make sense to check for NULL, print an error and then
dereference the pointer. The normal Oops message is easy to debug
without the extra code added here. Or maybe it is possible for
the pointer to be NULL in that case we should handle it
correctly. ]
Hi Hiral,
FYI, there are new smatch warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
head: e3ff197a750d2912d0bb2a0161c23c18bad250ad
commit: 188061001ac78b40780af042dd2156e2213e29ed [23/27] [SCSI] fnic:fixing
issues in device and firmware reset code
drivers/scsi/fnic/fnic_scsi.c:404 fnic_queuecommand_lck() error: potential
NULL dereference 'rport'.
drivers/scsi/fnic/fnic_scsi.c:303 fnic_queue_wq_copy_desc() error: potential
NULL dereference 'rport'.
+ drivers/scsi/fnic/fnic_scsi.c:1260 fnic_rport_exch_reset() error: we
previously assumed 'sc-device' could be null (see line 1237)
+ drivers/scsi/fnic/fnic_scsi.c:1325 fnic_terminate_rport_io() error: we
previously assumed 'sc-device' could be null (see line 1314)
drivers/scsi/fnic/fnic_scsi.c:1431 fnic_abort_cmd() error: potential NULL
dereference 'rport'.
drivers/scsi/fnic/fnic_scsi.c:1787 fnic_device_reset() error: potential NULL
dereference 'rport'.
git remote add scsi git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git
git remote update scsi
git checkout 188061001ac78b40780af042dd2156e2213e29ed
vim +1260 drivers/scsi/fnic/fnic_scsi.c
18806100 Hiral Patel 2012-12-10 1231 if (io_req-abts_done) {
18806100 Hiral Patel 2012-12-10 1232
shost_printk(KERN_ERR, fnic-lport-host,
18806100 Hiral Patel 2012-12-10 1233
fnic_rport_exch_reset: io_req-abts_done is set
18806100 Hiral Patel 2012-12-10 1234 state is %s\n,
18806100 Hiral Patel 2012-12-10 1235
fnic_ioreq_state_to_str(CMD_STATE(sc)));
18806100 Hiral Patel 2012-12-10 1236 }
18806100 Hiral Patel 2012-12-10 @1237 if (sc-device == NULL)
18806100 Hiral Patel 2012-12-10 1238
shost_printk(KERN_ERR, fnic-lport-host,
18806100 Hiral Patel 2012-12-10 1239
fnic_rport_exch_reset: sc-device is null state is
18806100 Hiral Patel 2012-12-10 1240 %s\n,
fnic_ioreq_state_to_str(CMD_STATE(sc)));
18806100 Hiral Patel 2012-12-10 1241
5df6d737 Abhijeet Joglekar 2009-04-17 1242 old_ioreq_state =
CMD_STATE(sc);
5df6d737 Abhijeet Joglekar 2009-04-17 1243 CMD_STATE(sc) =
FNIC_IOREQ_ABTS_PENDING;
5df6d737 Abhijeet Joglekar 2009-04-17 1244 CMD_ABTS_STATUS(sc) =
FCPIO_INVALID_CODE;
18806100 Hiral Patel 2012-12-10 1245 if (CMD_FLAGS(sc)
FNIC_DEVICE_RESET) {
18806100 Hiral Patel 2012-12-10 1246 abt_tag = (tag
| FNIC_TAG_DEV_RST);
18806100 Hiral Patel 2012-12-10 1247
FNIC_SCSI_DBG(KERN_DEBUG, fnic-lport-host,
18806100 Hiral Patel 2012-12-10 1248
fnic_rport_exch_reset dev rst sc 0x%p\n,
18806100 Hiral Patel 2012-12-10 1249 sc);
18806100 Hiral Patel 2012-12-10 1250 }
5df6d737 Abhijeet Joglekar 2009-04-17 1251
5df6d737 Abhijeet Joglekar 2009-04-17 1252
BUG_ON(io_req-abts_done);
5df6d737 Abhijeet Joglekar 2009-04-17 1253
5df6d737 Abhijeet Joglekar 2009-04-17 1254
FNIC_SCSI_DBG(KERN_DEBUG, fnic-lport-host,
5df6d737 Abhijeet Joglekar 2009-04-17 1255
fnic_rport_reset_exch: Issuing abts\n);
5df6d737 Abhijeet Joglekar 2009-04-17 1256
5df6d737 Abhijeet Joglekar 2009-04-17 1257
spin_unlock_irqrestore(io_lock, flags);
5df6d737 Abhijeet Joglekar 2009-04-17 1258
5df6d737 Abhijeet Joglekar 2009-04-17 1259 /* Now queue the abort
command to firmware */
5df6d737 Abhijeet Joglekar 2009-04-17 @1260
int_to_scsilun(sc-device-lun, fc_lun);
5df6d737 Abhijeet Joglekar 2009-04-17 1261
18806100 Hiral Patel 2012-12-10 1262 if
(fnic_queue_abort_io_req(fnic, abt_tag,
5df6d737 Abhijeet Joglekar 2009-04-17 1263
FCPIO_ITMF_ABT_TASK_TERM,
5df6d737 Abhijeet Joglekar 2009-04-17 1264
fc_lun.scsi_lun, io_req)) {
5df6d737 Abhijeet Joglekar 2009-04-17 1265 /*
5df6d737 Abhijeet Joglekar 2009-04-17 1266 * Revert the
cmd state back to old state, if
25985edc Lucas De Marchi 2011-03-30 1267 * it hasn't
changed in between. This cmd will get
5df6d737 Abhijeet Joglekar 2009-04-17 1268 * aborted
later by scsi_eh, or cleaned up during
5df6d737 Abhijeet Joglekar 2009-04-17 1269 * lun reset
5df6d737 Abhijeet Joglekar 2009-04-17 1270 */
5df6d737 Abhijeet
[patch] [SCSI] bfa: fix strncpy() limiter in bfad_start_ops()
The closing parenthesis is in the wrong place so it takes the sizeof a
pointer instead of the sizeof the buffer minus one.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index e6bf126..a5f7690 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -1034,7 +1034,7 @@ bfad_start_ops(struct bfad_s *bfad) {
sizeof(driver_info.host_os_patch) - 1);
strncpy(driver_info.os_device_name, bfad-pci_name,
- sizeof(driver_info.os_device_name - 1));
+ sizeof(driver_info.os_device_name) - 1);
/* FCS driver info init */
spin_lock_irqsave(bfad-bfad_lock, flags);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] target: change sprintf to snprintf
buf is 128 characters and vpd-device_identifier is 256. It makes
the static checkers complain.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Probably we should just raise VPD_TMP_BUF_SIZE to 300.
diff --git a/drivers/target/target_core_transport.c
b/drivers/target/target_core_transport.c
index bd587b7..96b64d5 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -907,15 +907,18 @@ int transport_dump_vpd_ident(
switch (vpd-device_identifier_code_set) {
case 0x01: /* Binary */
- sprintf(buf, T10 VPD Binary Device Identifier: %s\n,
+ snprintf(buf, sizeof(buf),
+ T10 VPD Binary Device Identifier: %s\n,
vpd-device_identifier[0]);
break;
case 0x02: /* ASCII */
- sprintf(buf, T10 VPD ASCII Device Identifier: %s\n,
+ snprintf(buf, sizeof(buf),
+ T10 VPD ASCII Device Identifier: %s\n,
vpd-device_identifier[0]);
break;
case 0x03: /* UTF-8 */
- sprintf(buf, T10 VPD UTF-8 Device Identifier: %s\n,
+ snprintf(buf, sizeof(buf),
+ T10 VPD UTF-8 Device Identifier: %s\n,
vpd-device_identifier[0]);
break;
default:
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] target: change sprintf to snprintf
On Fri, Jan 18, 2013 at 11:36:52AM -0800, Nicholas A. Bellinger wrote: On Fri, 2013-01-18 at 16:05 +0300, Dan Carpenter wrote: buf is 128 characters and vpd-device_identifier is 256. It makes the static checkers complain. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Applied to target-pending/for-next. Probably we should just raise VPD_TMP_BUF_SIZE to 300. Also bumping up VPD_TMP_BUF_SIZE to 254 to match the usage with INQUIRY_VPD_DEVICE_IDENTIFIER_LEN sized vpd-device_identifier[] below. It could still go over 254 because it's T10 VPD Binary Device Identifier: plus INQUIRY_VPD_DEVICE_IDENTIFIER_LEN. But I agree that probably 254 is enough space. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] libosd: check for kzalloc() failure
There wasn't any error handling for this kzalloc().
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index c06b8e5..d8293f2 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev *od,
odi-osdname_len = get_attrs[a].len;
/* Avoid NULL for memcmp optimization 0-length is good enough */
odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
+ if (!odi-osdname) {
+ ret = -ENOMEM;
+ goto out;
+ }
if (odi-osdname_len)
memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
OSD_INFO(OSD_NAME [%s]\n, odi-osdname);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [SCSI] libosd: check for kzalloc() failure
On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote:
Am 30.01.2013 08:06, schrieb Dan Carpenter:
There wasn't any error handling for this kzalloc().
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/osd/osd_initiator.c
b/drivers/scsi/osd/osd_initiator.c
index c06b8e5..d8293f2 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev
*od,
odi-osdname_len = get_attrs[a].len;
/* Avoid NULL for memcmp optimization 0-length is good enough */
odi-osdname = kzalloc(odi-osdname_len + 1, GFP_KERNEL);
+ if (!odi-osdname) {
+ ret = -ENOMEM;
+ goto out;
+ }
if (odi-osdname_len)
memcpy(odi-osdname, get_attrs[a].val_ptr, odi-osdname_len);
OSD_INFO(OSD_NAME [%s]\n, odi-osdname);
--
this looks like strdup() ?
Maybe? It's a funny thing going on with the NUL terminator and I
don't understand what the comment is about.
It appears that normally get_attrs[a].val_ptr is a NUL terminated
string but get_attrs[a].len does not count the terminator.
Odd.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/2] target: don't always say ipv6 as address type
lstat-last_intr_fail_ip_addr is an array inside the lstat struct. It's never NULL so we always print ipv6\n here. The test should be if (lstat-last_intr_fail_ip_family == AF_INET6). We don't need the temporary buffer either. We could print directly into page. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Compile tested only. diff --git a/drivers/target/iscsi/iscsi_target_stat.c b/drivers/target/iscsi/iscsi_target_stat.c index 421d694..54a6f38 100644 --- a/drivers/target/iscsi/iscsi_target_stat.c +++ b/drivers/target/iscsi/iscsi_target_stat.c @@ -410,14 +410,16 @@ static ssize_t iscsi_stat_tgt_attr_show_attr_fail_intr_addr_type( struct iscsi_tiqn *tiqn = container_of(igrps, struct iscsi_tiqn, tiqn_stat_grps); struct iscsi_login_stats *lstat = tiqn-login_stats; - unsigned char buf[8]; + int ret; spin_lock(lstat-lock); - snprintf(buf, 8, %s, (lstat-last_intr_fail_ip_addr != NULL) ? - ipv6 : ipv4); + if (lstat-last_intr_fail_ip_family == AF_INET6) + ret = snprintf(page, PAGE_SIZE, ipv6\n); + else + ret = snprintf(page, PAGE_SIZE, ipv4\n); spin_unlock(lstat-lock); - return snprintf(page, PAGE_SIZE, %s\n, buf); + return ret; } ISCSI_STAT_TGT_ATTR_RO(fail_intr_addr_type); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/2] target: don't truncate the fail intr address
The temporary buffer was only 32 characters but -last_intr_fail_ip_addr
is a 48 character buffer. We don't need to use a temporary buffer at
all, we can just print directly to page.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Compile tested only.
diff --git a/drivers/target/iscsi/iscsi_target_stat.c
b/drivers/target/iscsi/iscsi_target_stat.c
index 54a6f38..464b420 100644
--- a/drivers/target/iscsi/iscsi_target_stat.c
+++ b/drivers/target/iscsi/iscsi_target_stat.c
@@ -429,16 +429,19 @@ static ssize_t
iscsi_stat_tgt_attr_show_attr_fail_intr_addr(
struct iscsi_tiqn *tiqn = container_of(igrps,
struct iscsi_tiqn, tiqn_stat_grps);
struct iscsi_login_stats *lstat = tiqn-login_stats;
- unsigned char buf[32];
+ int ret;
spin_lock(lstat-lock);
- if (lstat-last_intr_fail_ip_family == AF_INET6)
- snprintf(buf, 32, [%s], lstat-last_intr_fail_ip_addr);
- else
- snprintf(buf, 32, %s, lstat-last_intr_fail_ip_addr);
+ if (lstat-last_intr_fail_ip_family == AF_INET6) {
+ ret = snprintf(page, PAGE_SIZE, [%s]\n,
+ lstat-last_intr_fail_ip_addr);
+ } else {
+ ret = snprintf(page, PAGE_SIZE, %s\n,
+ lstat-last_intr_fail_ip_addr);
+ }
spin_unlock(lstat-lock);
- return snprintf(page, PAGE_SIZE, %s\n, buf);
+ return ret;
}
ISCSI_STAT_TGT_ATTR_RO(fail_intr_addr);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] be2iscsi: WRB Initialization and Failure code path change
Hello Jayamohan,
I had a question about 3ec7827134a9: [SCSI] be2iscsi: WRB
Initialization and Failure code path change from Apr 3, 2012.
drivers/scsi/be2iscsi/be_main.c
2683 for (index = 0; index phba-params.cxns_per_ctrl * 2; index
+= 2) {
^^
We are allocating every other element in the array. Why?
2684 pwrb_context = phwi_ctrlr-wrb_context[index];
2685 pwrb_context-pwrb_handle_base =
2686 kzalloc(sizeof(struct wrb_handle *) *
2687 phba-params.wrbs_per_cxn,
GFP_KERNEL);
2688 if (!pwrb_context-pwrb_handle_base) {
2689 beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
2690 BM_%d : Mem Alloc Failed. Failing
to load\n);
2691 goto init_wrb_hndl_failed;
2692 }
[snip]
2746 init_wrb_hndl_failed:
2747 for (j = index; j 0; j--) {
^
Here we are freeing every element in the array except -wrb_context[0].
Some of the elements were not allocated, and doesn't skipping zero lead
to a memory leak?
2748 pwrb_context = phwi_ctrlr-wrb_context[j];
2749 kfree(pwrb_context-pwrb_handle_base);
2750 kfree(pwrb_context-pwrb_handle_basestd);
2751 }
2752 return -ENOMEM;
2753 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] lpfc 8.3.37: Provide support for FCoE protocol dual-chute (ULP) operation
Hello James Smart,
The patch 962bc51b04b2: [SCSI] lpfc 8.3.37: Provide support for FCoE
protocol dual-chute (ULP) operation from Jan 3, 2013, leads to the
following warning:
drivers/scsi/lpfc/lpfc_sli.c:12818 lpfc_wq_create()
warn: 0x800 is larger than 8 bits
[ This is from a not ready for release Smatch check. ]
drivers/scsi/lpfc/lpfc_sli.c
12818 if (phba-sli4_hba.fw_func_mode LPFC_DUA_MODE)
^^^
fw_func_mode is unsigned char.
LPFC_DUA_MODE is 0x800.
This condition is always false because 0xff 0x800 is zero.
12819 bf_set(lpfc_mbx_wq_create_dua, wq_create-u.request,
1);
12820
12821 rc = lpfc_sli_issue_mbox(phba, mbox, MBX_POLL);
12822 /* The IOCTL status is embedded in the mailbox subheader. */
12823 shdr_status = bf_get(lpfc_mbox_hdr_status, shdr-response);
12824 shdr_add_status = bf_get(lpfc_mbox_hdr_add_status,
shdr-response);
12825 if (shdr_status || shdr_add_status || rc) {
12826 lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
12827 2503 WQ_CREATE mailbox failed with
12828 status x%x add_status x%x, mbx status
x%x\n,
12829 shdr_status, shdr_add_status, rc);
12830 status = -ENXIO;
12831 goto out;
12832 }
12833 wq-queue_id = bf_get(lpfc_mbx_wq_create_q_id,
wq_create-u.response);
12834 if (wq-queue_id == 0x) {
12835 status = -ENXIO;
12836 goto out;
12837 }
12838 if (phba-sli4_hba.fw_func_mode LPFC_DUA_MODE) {
^^^
Same.
12839 wq-db_format = bf_get(lpfc_mbx_wq_create_db_format,
12840 wq_create-u.response);
12841 if ((wq-db_format != LPFC_DB_LIST_FORMAT)
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] csiostor: Chelsio FCoE offload driver
Hopefully, you recieved an email about this last November, but this
is a follow up because the bug is still there.
Smatch complains about a buffer overflow in this:
drivers/scsi/csiostor/csio_rnode.c:872 csio_rnode_fwevt_handler()
error: buffer overflow '(rn)-stats.n_evt_fw' 22 = 26
859 void
860 csio_rnode_fwevt_handler(struct csio_rnode *rn, uint8_t fwevt)
861 {
862 struct csio_lnode *ln = csio_rnode_to_lnode(rn);
863 enum csio_rn_ev evt;
864
865 evt = CSIO_FWE_TO_RNFE(fwevt);
866 if (!evt) {
Events greater than PROTO_ERR_IMPL_LOGO are invalid.
867 csio_ln_err(ln, ssni:x%x Unhandled FW Rdev event:
%d\n,
868 csio_rn_flowid(rn), fwevt);
869 CSIO_INC_STATS(rn, n_evt_unexp);
870 return;
871 }
872 CSIO_INC_STATS(rn, n_evt_fw[fwevt]);
It looks like new events were added and the size of the n_evt_fw[]
array wasn't updated to hold them. Everything after RSCN_DEV_LOST
causes memory corruption.
RSCN_DEV_LOST = 0x16,
SCR_ACC_RCVD= 0x17,
ADISC_RJT_RCVD = 0x18,
LOGO_SNT= 0x19,
PROTO_ERR_IMPL_LOGO = 0x1a,
There is a related bug in the lnode version of this code which
Smatch does not catch.
drivers/scsi/csiostor/csio_lnode.c
1555 /* save previous event for debugging */
1556 ln-prev_evt = ln-cur_evt;
1557 ln-cur_evt = rdev_wr-event_cause;
1558 CSIO_INC_STATS(ln,
n_evt_fw[rdev_wr-event_cause]);
^^
Memory corruption.
1559
1560 /* Translate all the fabric events to lnode SM
events */
1561 evt = CSIO_FWE_TO_LNE(rdev_wr-event_cause);
1562 if (evt) {
Valid events handled here but we already corrupted memory three
lines earlier.
1563 csio_ln_dbg(ln,
1564 Posting event to lnode
event:%d
1565 cause:%d flowid:x%x\n, evt,
1566 rdev_wr-event_cause,
rdev_flowid);
1567 csio_post_event(ln-sm, evt);
1568 }
1569
I wasn't a part of the discussion in November, but the fix for this
seems trivial. I'm probably missing something?
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI] sd: Ensure we correctly disable devices with unknown protection type
This patch wasn't applied. Probably because it didn't have [PATCH] in the email subject. regards, dan carpenter On Wed, Sep 26, 2012 at 10:39:44PM -0400, Martin K. Petersen wrote: Dan == Dan Carpenter dan.carpen...@oracle.com writes: Dan, Dan warn: buffer overflow 'cap' 4 = 4 Argh, yes. Type 3 is 4 because it's a bitmask. -- Martin K. PetersenOracle Linux Engineering SCSI: Fix range check in scsi_host.h The range checking from fe542396 was bad. We would still end up walking beyond the array as Type 3 is defined to be 4 in the protection bitmask. Instead use ARRAY_SIZE() for the range check. Reported-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Martin K. Petersen martin.peter...@oracle.com diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 4908480..2b6956e 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -873,7 +873,7 @@ static inline unsigned int scsi_host_dif_capable(struct Scsi_Host *shost, unsign SHOST_DIF_TYPE2_PROTECTION, SHOST_DIF_TYPE3_PROTECTION }; - if (target_type SHOST_DIF_TYPE3_PROTECTION) + if (target_type = ARRAY_SIZE(cap)) return 0; return shost-prot_capabilities cap[target_type] ? target_type : 0; @@ -887,7 +887,7 @@ static inline unsigned int scsi_host_dix_capable(struct Scsi_Host *shost, unsign SHOST_DIX_TYPE2_PROTECTION, SHOST_DIX_TYPE3_PROTECTION }; - if (target_type SHOST_DIX_TYPE3_PROTECTION) + if (target_type = ARRAY_SIZE(cap)) return 0; return shost-prot_capabilities cap[target_type]; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] dc395x: uninitialized variable in device_alloc()
This bug was introduced back in bitkeeper days in 2003. We use dcb-dev_mode before it has been initialized. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c index 865c64f..fed486bf 100644 --- a/drivers/scsi/dc395x.c +++ b/drivers/scsi/dc395x.c @@ -3747,13 +3747,13 @@ static struct DeviceCtlBlk *device_alloc(struct AdapterCtlBlk *acb, dcb-max_command = 1; dcb-target_id = target; dcb-target_lun = lun; + dcb-dev_mode = eeprom-target[target].cfg0; #ifndef DC395x_NO_DISCONNECT dcb-identify_msg = IDENTIFY(dcb-dev_mode NTC_DO_DISCONNECT, lun); #else dcb-identify_msg = IDENTIFY(0, lun); #endif - dcb-dev_mode = eeprom-target[target].cfg0; dcb-inquiry7 = 0; dcb-sync_mode = 0; dcb-min_nego_period = clock_period[period_index]; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] cxgb4i: a couple invalid dereference bugs
tid and opc come from skb-data so Smatch complains that the
bounds checking is inadequate. If tid is out of bounds it would
lead to a NULL dereference when debugging is enabled. If opc is
invalid we would dereference a bogus function pointer.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 3fecf35..4facd10 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -1047,8 +1047,11 @@ static void do_set_tcb_rpl(struct cxgbi_device *cdev,
struct sk_buff *skb)
struct cxgbi_sock *csk;
csk = lookup_tid(t, tid);
- if (!csk)
+ if (!csk) {
pr_err(can't find conn. for tid %u.\n, tid);
+ __kfree_skb(skb);
+ return;
+ }
log_debug(1 CXGBI_DBG_TOE | 1 CXGBI_DBG_SOCK,
csk 0x%p,%u,%lx,%u, status 0x%x.\n,
@@ -1534,7 +1537,7 @@ static int t4_uld_rx_handler(void *handle, const __be64
*rsp,
log_debug(1 CXGBI_DBG_TOE,
cdev %p, opcode 0x%x(0x%x,0x%x), skb %p.\n,
cdev, opc, rpl-ot.opcode_tid, ntohl(rpl-ot.opcode_tid), skb);
- if (cxgb4i_cplhandlers[opc])
+ if (opc ARRAY_SIZE(cxgb4i_cplhandlers) cxgb4i_cplhandlers[opc])
cxgb4i_cplhandlers[opc](cdev, skb);
else {
pr_err(No handler for opcode 0x%x.\n, opc);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] bfa: Use GFP_ATOMIC under spin_lock
This is always called with spinlocks held so it should use
GFP_ATOMIC. The call tree is:
- bfad_drv_start()
Takes spin_lock_irqsave(bfad-bfad_lock, flags);
- bfa_fcs_pbc_vport_init()
- bfa_fcb_pbc_vport_create()
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index a5f7690..d144a06 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -491,7 +491,7 @@ bfa_fcb_pbc_vport_create(struct bfad_s *bfad, struct
bfi_pbc_vport_s pbc_vport)
struct bfad_vport_s *vport;
int rc;
- vport = kzalloc(sizeof(struct bfad_vport_s), GFP_KERNEL);
+ vport = kzalloc(sizeof(struct bfad_vport_s), GFP_ATOMIC);
if (!vport) {
bfa_trc(bfad, 0);
return;
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] scsi_transport_sas: check for allocation failure
Static checkers complain that this allocation isn't checked. We
should return zero if the allocation fails.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index 1b68142..a022997 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -379,9 +379,12 @@ sas_tlr_supported(struct scsi_device *sdev)
{
const int vpd_len = 32;
struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
- char *buffer = kzalloc(vpd_len, GFP_KERNEL);
+ char *buffer;
int ret = 0;
+ buffer = kzalloc(vpd_len, GFP_KERNEL);
+ if (!buffer)
+ goto out;
if (scsi_get_vpd_page(sdev, 0x90, buffer, vpd_len))
goto out;
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [SCSI] scsi_transport_sas: check for allocation failure
On Fri, Mar 08, 2013 at 12:57:12PM -0500, Douglas Gilbert wrote: On 13-03-08 07:02 AM, Dan Carpenter wrote: For 32 bytes, why not use the stack? It's a good point. I'll resend on Monday. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] mpt3sas: move dereference under check
pci_pool_free() dereferences ioc-sense_dma_pool but we check it
for NULL on the following line.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 1836003..06a84ef 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2479,9 +2479,11 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
}
if (ioc-sense) {
- pci_pool_free(ioc-sense_dma_pool, ioc-sense, ioc-sense_dma);
- if (ioc-sense_dma_pool)
+ if (ioc-sense_dma_pool) {
+ pci_pool_free(ioc-sense_dma_pool, ioc-sense,
+ ioc-sense_dma);
pci_pool_destroy(ioc-sense_dma_pool);
+ }
dexitprintk(ioc, pr_info(MPT3SAS_FMT
sense_pool(0x%p): free\n,
ioc-name, ioc-sense));
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [SCSI] scsi_transport_sas: check for allocation failure
On Fri, Mar 08, 2013 at 10:50:19PM +, James Bottomley wrote:
On Fri, 2013-03-08 at 12:57 -0500, Douglas Gilbert wrote:
On 13-03-08 07:02 AM, Dan Carpenter wrote:
Static checkers complain that this allocation isn't checked. We
should return zero if the allocation fails.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index 1b68142..a022997 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -379,9 +379,12 @@ sas_tlr_supported(struct scsi_device *sdev)
{
const int vpd_len = 32;
struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
- char *buffer = kzalloc(vpd_len, GFP_KERNEL);
+ char *buffer;
int ret = 0;
+ buffer = kzalloc(vpd_len, GFP_KERNEL);
+ if (!buffer)
+ goto out;
if (scsi_get_vpd_page(sdev, 0x90, buffer, vpd_len))
goto out;
For 32 bytes, why not use the stack?
Because the buffer is a DMA target. You can't DMA to stack because of
padding and cacheline issues.
I think stack data works here. scsi_execute() calls
blk_rq_map_kern() which handles stack memory and alignment issues.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] eata_pio: off by one in eata_pio_detect()
Smatch complains that the reg_IRQ[] array only has MAXIRQ (16) elements so we are one space beyond the end of the array here. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/scsi/eata_pio.c b/drivers/scsi/eata_pio.c index 356def4..1663173 100644 --- a/drivers/scsi/eata_pio.c +++ b/drivers/scsi/eata_pio.c @@ -919,7 +919,7 @@ static int eata_pio_detect(struct scsi_host_template *tpnt) find_pio_EISA(gc); find_pio_ISA(gc); - for (i = 0; i = MAXIRQ; i++) + for (i = 0; i MAXIRQ; i++) if (reg_IRQ[i]) request_irq(i, do_eata_pio_int_handler, IRQF_DISABLED, EATA-PIO, NULL); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] arcmsr: Support Areca new SATA Raid Adapter ARC1214/1224/1264/1284
Hello 黃清隆,
The patch 17628f3a062b: [SCSI] arcmsr: Support Areca new SATA Raid
Adapter ARC1214/1224/1264/1284 from Aug 26, 2013, leads to the
following Smatch warning:
drivers/scsi/arcmsr/arcmsr_hba.c:3580 arcmsr_hbaD_get_config()
warn: signedness bug returning '(-12)'
drivers/scsi/arcmsr/arcmsr_hba.c
3576 dma_coherent = dma_alloc_coherent(pdev-dev, acb-uncache_size,
3577 dma_coherent_handle, GFP_KERNEL);
3578 if (!dma_coherent) {
3579 pr_notice(DMA allocation failed...\n);
3580 return -ENOMEM;
^^
This should be returning false.
3581 }
Line 3577 has messed up indenting.
Also this patch says it adds support for new hardware but almost 900
lines out of this 3605 line patch are white space changes. Do the
unrelated white space changes in a separate patch.
This patch also re-introduces a bug which I fixed in the mainline kernel
a year ago.
drivers/scsi/arcmsr/arcmsr_hba.c
4525 writel(0xD, pmuC-write_sequence);
4526 } while temp = readl(pmuC-host_diagnostic)) |
^
This should be a '' not a '|'. Please fix this again back to the way
it was.
4527 ARCMSR_ARC1880_DiagWrite_ENABLE) == 0)
4528 (count 5));
The indenting here is messed up as well. This is a very low quality
patch.
I think you are not using git internally in your company and that is why
you are messing up so badly. Please learn to use it. Keep track of the
fixes which go into the mainline kernel. Separate the white space
cleanups from the new features.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
Hello Bradley Grove,
This is a semi-automatic email about new static checker warnings.
The patch 17adeb6dabbe: [SCSI] esas2r: ATTO Technology ExpressSAS 6G
SAS/SATA RAID Adapter Driver from Aug 23, 2013, leads to the
following Smatch complaint:
drivers/scsi/esas2r/esas2r_init.c:671 esas2r_cleanup()
warn: variable dereferenced before check 'host' (see line 668)
drivers/scsi/esas2r/esas2r_init.c
667 {
668 struct esas2r_adapter *a = (struct esas2r_adapter
*)host-hostdata;
^^
Patch adds dereference.
669 int index;
670
671 if (host == NULL) {
Patch adds check.
672 int i;
673
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver
Hello Bradley Grove, The patch 17adeb6dabbe: [SCSI] esas2r: ATTO Technology ExpressSAS 6G SAS/SATA RAID Adapter Driver from Aug 23, 2013, leads to the following Smatch warning: drivers/scsi/esas2r/esas2r_vda.c:312 esas2r_complete_vda_ioctl() error: format string overflow. buf_size: 4 length: 5 drivers/scsi/esas2r/esas2r_vda.c 312 sprintf((char *)cfg-data.init.fw_release, ^ This is a u32 but we are writing 4 characters and a NUL so it ends up putting the NUL in cfg-data.init.epoch_time. 313 %1d.%02d, 314 (int)LOBYTE(le16_to_cpu(rsp-fw_release)), 315 (int)HIBYTE(le16_to_cpu(rsp-fw_release))); regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] esas2r: bitwise vs logical AND typo
The intent was to test if any bits were set but it used a logical AND.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/esas2r/esas2r_init.c
b/drivers/scsi/esas2r/esas2r_init.c
index 3a798e7..78b18c1 100644
--- a/drivers/scsi/esas2r/esas2r_init.c
+++ b/drivers/scsi/esas2r/esas2r_init.c
@@ -808,7 +808,7 @@ static void esas2r_init_pci_cfg_space(struct esas2r_adapter
*a)
int pcie_cap_reg;
pcie_cap_reg = pci_find_capability(a-pcid, PCI_CAP_ID_EXP);
- if (0x pcie_cap_reg) {
+ if (0x pcie_cap_reg) {
u16 devcontrol;
pci_read_config_word(a-pcid, pcie_cap_reg + PCI_EXP_DEVCTL,
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SCSI] arcmsr: Support Areca new SATA Raid Adapter ARC1214/1224/1264/1284
One tool for breaking patches up is git citool highlight what you want to add to the patch and then right click and select stage lines for commit. So for example let's go through the changes to drivers/scsi/arcmsr/arcmsr.h. These should be broken up into multiple patches with the easiest whitespace patches first. The original patch adds 520 lines of code (I'm looking at the plus marks in git show 17628f3a062b | diffstat). Out of those 392 line are pure white space changes such as tabs or indents. 9 lines are adding parenthesis around macros. 4 lines are changing int32_t to uint32_t. It's not clear if there was a signedness bug in the original code or if this is purely cosmetic. That should be mentioned in the changelog. The remaining 115 lines are related to the new feature. So this should be a series of patches. [patch 1/4] whitespace (this is the largest patch in the series). [patch 2/4] add parenthesis [patch 3/4] change signed to unsigned [patch 4/4] add feature (this would touch other the other files as well) The first 3 patches are trivial to review and then the last one is much smaller and easier to review than when everything was mixed together. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] qla2xxx: Add support for ISP8044.
Hello Atul Deshmukh,
This is a semi-automatic email about new static checker warnings.
The patch ef4647420025: [SCSI] qla2xxx: Add support for ISP8044.
from Aug 27, 2013, leads to the following Smatch complaint:
drivers/scsi/qla2xxx/qla_os.c:2962 qla2x00_probe_one()
error: we previously assumed 'base_vha' could be null (see line 2632)
drivers/scsi/qla2xxx/qla_os.c
2631 base_vha = qla2x00_create_host(sht, ha);
2632 if (!base_vha) {
Existing check.
2633 ret = -ENOMEM;
2634 qla2x00_mem_free(ha);
2635 qla2x00_free_req_que(ha, req);
2636 qla2x00_free_rsp_que(ha, rsp);
2637 goto probe_hw_failed;
2638 }
[ snip ]
2954 probe_hw_failed:
2955 if (IS_QLA82XX(ha)) {
2956 qla82xx_idc_lock(ha);
2957 qla82xx_clear_drv_active(ha);
2958 qla82xx_idc_unlock(ha);
2959 }
2960 if (IS_QLA8044(ha)) {
2961 qla8044_idc_lock(ha);
2962 qla8044_clear_drv_active(base_vha);
Patch adds dereference.
2963 qla8044_idc_unlock(ha);
2964 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] atp870u: 64 bit bug in probe()
On 64 bit CPUs there is a memory corruption bug on probe(). It should
be setting 32 bits of data on both 32 and 64 bit.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Static checker stuff. I can't test this.
diff --git a/drivers/scsi/atp870u.c b/drivers/scsi/atp870u.c
index 15a629d..3edbf30 100644
--- a/drivers/scsi/atp870u.c
+++ b/drivers/scsi/atp870u.c
@@ -2791,11 +2791,11 @@ next_fblk_885:
p-global_map[m]= 0;
for (k=0; k 4; k++) {
outw(n++,base_io + 0x3c);
- ((unsigned long *)setupdata[m][0])[k]=inl(base_io +
0x38);
+ ((u32 *)setupdata[m][0])[k]=inl(base_io + 0x38);
}
for (k=0; k 4; k++) {
outw(n++,base_io + 0x3c);
- ((unsigned long *)p-sp[m][0])[k]=inl(base_io + 0x38);
+ ((u32 *)p-sp[m][0])[k]=inl(base_io + 0x38);
}
n += 8;
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference
If fibsize is zero then it leads to a ZERO_SIZE_PTR dereference when
we dereference user_srbcmd.
Due to a missing capable() check in the compat ioctls then this error
can be triggered without CAP_SYS_RAWIO. I have fixed that in a separate
patch.
Reported-by: Nico Golde n...@ngolde.de
Reported-by: Fabian Yamaguchi f...@goesec.de
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index d85ac1a..efd0ba3 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -511,7 +511,8 @@ static int aac_send_raw_srb(struct aac_dev* dev, void
__user * arg)
goto cleanup;
}
- if (fibsize (dev-max_fib_size - sizeof(struct aac_fibhdr))) {
+ if (fibsize == 0 ||
+ fibsize (dev-max_fib_size - sizeof(struct aac_fibhdr))) {
rcode = -EINVAL;
goto cleanup;
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] aacraid: missing capable() check in compat ioctl
In d496f94d22d1 ('[SCSI] aacraid: fix security weakness') we added a
check on CAP_SYS_RAWIO to the ioctl. The compat ioctls need the check
as well.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 408a42e..f0d432c 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -771,6 +771,8 @@ static long aac_compat_do_ioctl(struct aac_dev *dev,
unsigned cmd, unsigned long
static int aac_compat_ioctl(struct scsi_device *sdev, int cmd, void __user
*arg)
{
struct aac_dev *dev = (struct aac_dev *)sdev-host-hostdata;
+ if (!capable(CAP_SYS_RAWIO))
+ return -EPERM;
return aac_compat_do_ioctl(dev, cmd, (unsigned long)arg);
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [SCSI] aacraid: prevent ZERO_SIZE_PTR dereference
You and James are right. It should be checking against the sizeof(). I will send a v2 tomorrow. Sorry about that. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] megaraid: missing bounds check in mimd_to_kioc()
pthru32-dataxferlen comes from the user so we need to check that it's
not too large so we don't overflow the buffer.
Reported-by: Nico Golde n...@ngolde.de
Reported-by: Fabian Yamaguchi f...@goesec.de
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Please review this carefully because I have not tested it.
diff --git a/drivers/scsi/megaraid/megaraid_mm.c
b/drivers/scsi/megaraid/megaraid_mm.c
index dfffd0f..a706927 100644
--- a/drivers/scsi/megaraid/megaraid_mm.c
+++ b/drivers/scsi/megaraid/megaraid_mm.c
@@ -486,6 +486,8 @@ mimd_to_kioc(mimd_t __user *umimd, mraid_mmadp_t *adp,
uioc_t *kioc)
pthru32-dataxferaddr = kioc-buf_paddr;
if (kioc-data_dir UIOC_WR) {
+ if (pthru32-dataxferlen kioc-xferlen)
+ return -EINVAL;
if (copy_from_user(kioc-buf_vaddr, kioc-user_data,
pthru32-dataxferlen)) {
return (-EFAULT);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] aacraid: prevent ZERO_SIZE_PTR dereference
On Thu, Oct 31, 2013 at 02:01:02PM +0530, Mahesh Rajashekhara wrote: It appears that driver runs into a problem here if fibsize is too small because we allocate user_srbcmd with fibsize size only but later we access it until user_srbcmd-sg.count to copy it over to srbcmd. Seems to be not correct to test (fibsize sizeof(*user_srbcmd)) because this structure already includes one sg element and this is not needed for commands without data. So, we would recommend to add the following (instead of test for fibsize == 0). Don't forget the reported by tags. Reported-by: Nico Golde n...@ngolde.de Reported-by: Fabian Yamaguchi f...@goesec.de regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] mpt fusion: declare some sizes as unsigned
In mptctl_do_mpt_command() we cap the upper bound of .maxSenseBytes but
we don't check for negative values:
if (karg.maxSenseBytes MPT_SENSE_BUFFER_SIZE)
I've fixed this by making the type unsigned and I changed the
surrounding types to match as well.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/message/fusion/mptctl.h b/drivers/message/fusion/mptctl.h
index d564cc9..9ea5a18 100644
--- a/drivers/message/fusion/mptctl.h
+++ b/drivers/message/fusion/mptctl.h
@@ -324,11 +324,11 @@ struct mpt_ioctl_command {
char__user *dataInBufPtr;
char__user *dataOutBufPtr;
char__user *senseDataPtr;
- int maxReplyBytes;
- int dataInSize;
- int dataOutSize;
- int maxSenseBytes;
- int dataSgeOffset;
+ u32 maxReplyBytes;
+ u32 dataInSize;
+ u32 dataOutSize;
+ u32 maxSenseBytes;
+ u32 dataSgeOffset;
charMF[1];
};
@@ -343,11 +343,11 @@ struct mpt_ioctl_command32 {
u32 dataInBufPtr;
u32 dataOutBufPtr;
u32 senseDataPtr;
- int maxReplyBytes;
- int dataInSize;
- int dataOutSize;
- int maxSenseBytes;
- int dataSgeOffset;
+ u32 maxReplyBytes;
+ u32 dataInSize;
+ u32 dataOutSize;
+ u32 maxSenseBytes;
+ u32 dataSgeOffset;
charMF[1];
};
#endif /*}*/
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] pm80xx: Print SAS address of IO failed device.
Hello Anand Kumar Santhanam,
This is a semi-automatic email about new static checker warnings.
The patch cb269c26ed02: [SCSI] pm80xx: Print SAS address of IO
failed device. from Sep 17, 2013, leads to the following Smatch
complaint:
drivers/scsi/pm8001/pm8001_hwi.c:2343 mpi_sata_completion()
error: we previously assumed 't-dev' could be null (see line 2319)
drivers/scsi/pm8001/pm8001_hwi.c
2318 if (t) {
2319 if (t-dev (t-dev-lldd_dev))
^
Exisiting check.
2320 pm8001_dev = t-dev-lldd_dev;
2321 } else {
2322 PM8001_FAIL_DBG(pm8001_ha,
2323 pm8001_printk(task null\n));
2324 return;
2325 }
2326
2327 if ((pm8001_dev !(pm8001_dev-id NCQ_READ_LOG_FLAG))
2328 unlikely(!t || !t-lldd_task || !t-dev)) {
^^^
Existing check.
2329 PM8001_FAIL_DBG(pm8001_ha,
2330 pm8001_printk(task or dev null\n));
2331 return;
2332 }
2333
2334 ts = t-task_status;
2335 if (!ts) {
2336 PM8001_FAIL_DBG(pm8001_ha,
2337 pm8001_printk(ts null\n));
2338 return;
2339 }
2340 /* Print sas address of IO failed device */
2341 if ((status != IO_SUCCESS) (status != IO_OVERFLOW)
2342 (status != IO_UNDERFLOW)) {
2343 if (!((t-dev-parent)
Patch introduces new unchecked dereference.
2344 (DEV_IS_EXPANDER(t-dev-parent-dev_type {
2345 for (i = 0 , j = 4; j = 7 i = 3; i++ , j++)
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] pm80xx: Print SAS address of IO failed device.
Hello Anand Kumar Santhanam,
This is a semi-automatic email about new static checker warnings.
The patch cb269c26ed02: [SCSI] pm80xx: Print SAS address of IO
failed device. from Sep 17, 2013, leads to the following Smatch
complaint:
drivers/scsi/pm8001/pm8001_hwi.c:2368 mpi_sata_completion()
error: we previously assumed 'pm8001_dev' could be null (see line 2327)
drivers/scsi/pm8001/pm8001_hwi.c
2326
2327 if ((pm8001_dev !(pm8001_dev-id NCQ_READ_LOG_FLAG))
^^
Existing check.
2328 unlikely(!t || !t-lldd_task || !t-dev)) {
2329 PM8001_FAIL_DBG(pm8001_ha,
2330 pm8001_printk(task or dev null\n));
2331 return;
2332 }
2333
2334 ts = t-task_status;
2335 if (!ts) {
2336 PM8001_FAIL_DBG(pm8001_ha,
2337 pm8001_printk(ts null\n));
2338 return;
2339 }
2340 /* Print sas address of IO failed device */
2341 if ((status != IO_SUCCESS) (status != IO_OVERFLOW)
2342 (status != IO_UNDERFLOW)) {
2343 if (!((t-dev-parent)
2344 (DEV_IS_EXPANDER(t-dev-parent-dev_type {
2345 for (i = 0 , j = 4; j = 7 i = 3; i++ , j++)
2346 sata_addr_low[i] =
pm8001_ha-sas_addr[j];
2347 for (i = 0 , j = 0; j = 3 i = 3; i++ , j++)
2348 sata_addr_hi[i] =
pm8001_ha-sas_addr[j];
2349 memcpy(temp_sata_addr_low, sata_addr_low,
2350 sizeof(sata_addr_low));
2351 memcpy(temp_sata_addr_hi, sata_addr_hi,
2352 sizeof(sata_addr_hi));
2353 temp_sata_addr_hi = (((temp_sata_addr_hi 24)
0xff)
2354 |((temp_sata_addr_hi
8)
2355 0xff) |
2356 ((temp_sata_addr_hi
8)
2357 0xff00) |
2358 ((temp_sata_addr_hi
24)
2359 0xff00));
2360 temp_sata_addr_low = temp_sata_addr_low
24)
2361 0xff) |
2362 ((temp_sata_addr_low
8)
2363 0xff) |
2364 ((temp_sata_addr_low
8)
2365 0xff00) |
2366 ((temp_sata_addr_low
24)
2367 0xff00)) +
2368
pm8001_dev-attached_phy +
Patch introduces new unchecked dereference.
2369 0x10);
2370 PM8001_FAIL_DBG(pm8001_ha,
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] qla2xxx: Add support for ISP8044.
Hello Atul Deshmukh,
This is a semi-automatic email about new static checker warnings.
The patch 7ec0effd30bb: [SCSI] qla2xxx: Add support for ISP8044.
from Aug 27, 2013, leads to the following Smatch complaint:
drivers/scsi/qla2xxx/qla_os.c:2958 qla2x00_probe_one()
error: we previously assumed 'base_vha' could be null (see line 2628)
drivers/scsi/qla2xxx/qla_os.c
2627 base_vha = qla2x00_create_host(sht, ha);
2628 if (!base_vha) {
^
Existing check.
2629 ret = -ENOMEM;
2630 qla2x00_mem_free(ha);
2631 qla2x00_free_req_que(ha, req);
2632 qla2x00_free_rsp_que(ha, rsp);
2633 goto probe_hw_failed;
2634 }
[ snip ]
2935 probe_failed:
2936 if (base_vha-timer_active)
2937 qla2x00_stop_timer(base_vha);
2938 base_vha-flags.online = 0;
2939 if (ha-dpc_thread) {
2940 struct task_struct *t = ha-dpc_thread;
2941
2942 ha-dpc_thread = NULL;
2943 kthread_stop(t);
2944 }
2945
2946 qla2x00_free_device(base_vha);
2947
2948 scsi_host_put(base_vha-host);
2949
2950 probe_hw_failed:
2951 if (IS_QLA82XX(ha)) {
2952 qla82xx_idc_lock(ha);
2953 qla82xx_clear_drv_active(ha);
2954 qla82xx_idc_unlock(ha);
2955 }
2956 if (IS_QLA8044(ha)) {
2957 qla8044_idc_lock(ha);
2958 qla8044_clear_drv_active(base_vha);
Patch introduces new unchecked dereference.
2959 qla8044_idc_unlock(ha);
2960 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] [SCSI] qla4xxx: overflow in qla4xxx_set_chap_entry()
We should cap the size of memcpy() because it comes from the network
and can't be trusted.
Fixes: 26ffd7b45fe9 ('[SCSI] qla4xxx: Add support to set CHAP entries')
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index a28d5e6..cf174a4 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -802,6 +802,7 @@ static int qla4xxx_set_chap_entry(struct Scsi_Host *shost,
void *data, int len)
int type;
int rem = len;
int rc = 0;
+ int size;
memset(chap_rec, 0, sizeof(chap_rec));
@@ -816,12 +817,14 @@ static int qla4xxx_set_chap_entry(struct Scsi_Host
*shost, void *data, int len)
chap_rec.chap_type = param_info-value[0];
break;
case ISCSI_CHAP_PARAM_USERNAME:
- memcpy(chap_rec.username, param_info-value,
- param_info-len);
+ size = min_t(size_t, sizeof(chap_rec.username),
+param_info-len);
+ memcpy(chap_rec.username, param_info-value, size);
break;
case ISCSI_CHAP_PARAM_PASSWORD:
- memcpy(chap_rec.password, param_info-value,
- param_info-len);
+ size = min_t(size_t, sizeof(chap_rec.password),
+param_info-len);
+ memcpy(chap_rec.password, param_info-value, size);
break;
case ISCSI_CHAP_PARAM_PASSWORD_LEN:
chap_rec.password_length = param_info-value[0];
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [SCSI] qla4xxx: overflow in qla4xxx_set_chap_entry()
On Wed, Nov 13, 2013 at 11:52:37AM +, Vikas Chaudhary wrote: -Original Message- From: Dan Carpenter dan.carpen...@oracle.com Date: Wednesday, 13 November 2013 1:18 pm To: Vikas vikas.chaudh...@qlogic.com, Adheer Chandravanshi adheer.chandravan...@qlogic.com Cc: Dept-Eng iSCSI Driver dept-iscsidri...@qlogic.com, James E.J. Bottomley jbottom...@parallels.com, scsi linux-scsi@vger.kernel.org, kernel-janit...@vger.kernel.org kernel-janit...@vger.kernel.org Subject: [patch] [SCSI] qla4xxx: overflow in qla4xxx_set_chap_entry() We should cap the size of memcpy() because it comes from the network and can't be trusted. This patch is on assumption that data is coming from network, but in this case data come from application (iscsiadm) with correct length. No, that doesn't work. We don't trust user space. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [SCSI] qla4xxx: overflow in qla4xxx_set_chap_entry()
On Wed, Nov 13, 2013 at 03:08:12PM +0300, Dan Carpenter wrote:
On Wed, Nov 13, 2013 at 11:52:37AM +, Vikas Chaudhary wrote:
-Original Message-
From: Dan Carpenter dan.carpen...@oracle.com
Date: Wednesday, 13 November 2013 1:18 pm
To: Vikas vikas.chaudh...@qlogic.com, Adheer Chandravanshi
adheer.chandravan...@qlogic.com
Cc: Dept-Eng iSCSI Driver dept-iscsidri...@qlogic.com, James E.J.
Bottomley jbottom...@parallels.com, scsi linux-scsi@vger.kernel.org,
kernel-janit...@vger.kernel.org kernel-janit...@vger.kernel.org
Subject: [patch] [SCSI] qla4xxx: overflow in qla4xxx_set_chap_entry()
We should cap the size of memcpy() because it comes from the network
and can't be trusted.
This patch is on assumption that data is coming from network,
but in this case data come from application (iscsiadm) with correct length.
No, that doesn't work. We don't trust user space.
Btw, the is especially true with network namespaces... These days
anyone who is ns_capable() could overflow the buffer after:
df008c91f835 ('net: Allow userns root to control llc, netfilter, netlink,
packet, and xfrm')
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] bfa: Firmware patch simplification
Hello Vijaya Mohan Guvva,
The patch 4e88a72d1574: [SCSI] bfa: Firmware patch simplification
from Nov 21, 2013, leads to the following Smatch warning:
drivers/scsi/bfa/bfa_ioc.c:6859 bfa_flash_status_read()
warn: unsigned 'status' is never less than zero.
drivers/scsi/bfa/bfa_ioc.c
6851 bfa_flash_status_read(void __iomem *pci_bar)
6852 {
6853 union bfa_flash_dev_status_reg_udev_status;
6854 u32 status;
6855 u32 ret_status;
6856 int i;
6857
6858 status = bfa_flash_fifo_flush(pci_bar);
6859 if (status 0)
^^
status is unsigned.
6860 return status;
There are some other Smatch warnings as well:
drivers/scsi/bfa/bfa_ioc.c:3882 bfa_sfp_show_comp() error: memcpy() 'des' too
small (64 vs 248)
This is not a real bug, it's just an over-sight. The code uses:
u8 *des = (u8 *) (sfp-sfpmem-srlid_base);
but the intent was:
void *dest = sfp-sfpmem;
The two assignments are equivalent in terms of pointer math, but we are
refering to the whole struct and not the first member.
The other warnings are real:
drivers/scsi/bfa/bfa_ioc.c:6859 bfa_flash_status_read() warn: unsigned 'status'
is never less than zero.
drivers/scsi/bfa/bfa_ioc.c:6881 bfa_flash_status_read() warn: unsigned 'status'
is never less than zero.
drivers/scsi/bfa/bfa_ioc.c:6917 bfa_flash_read_start() warn: unsigned 'status'
is never less than zero.
drivers/scsi/bfa/bfa_ioc.c:7043 bfa_flash_raw_read() warn: unsigned 'status' is
never less than zero.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [SCSI] fnic: FIP VLAN Discovery Feature Support
-desc.wwnn.fd_desc.fip_dtype = FIP_DT_NAME; 339 vlan-desc.wwnn.fd_desc.fip_dlen = sizeof(vlan-desc.wwnn) / FIP_BPW; ^ This is actually a second Smatch bug which it inherited from Sparse. It doesn't understand packed structs correctly and think we are setting -fib_dlen to 4 but we are setting it to 3. 340 put_unaligned_be64(fip-lp-wwnn, vlan-desc.wwnn.fd_wwn); 341 atomic64_inc(fnic_stats-vlan_stats.vlan_disc_reqs); 342 I suspect that the size calculation is not taking sizeof(struct fc_frame_header) into consideration properly in fnic_fcoe_send_vlan_req()? regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
