public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [patch -resend] [SCSI] bfa: off by one in bfa_ioc_mbox_isr()
       [not found] <20120627085800.GA3007@mwanda>
@ 2012-06-27  8:59 ` Dan Carpenter
  2012-06-27 17:44   ` Krishna Gudipati
  2012-06-27  8:59 ` [patch -resend] [SCSI] bfa: dereferencing freed memory in bfad_im_probe() Dan Carpenter
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  8:59 UTC (permalink / raw)
  To: Jing Huang
  Cc: Krishna C Gudipati, James E.J. Bottomley, linux-scsi,
	linux-kernel, kernel-janitors

If mc == BFI_MC_MAX then we're reading past the end of the
mod->mbhdlr[] array.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Originally sent on Wed, 6 Jul 2011.

diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index 14e6284..8cdb79c 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -2357,7 +2357,7 @@ bfa_ioc_mbox_isr(struct bfa_ioc_s *ioc)
 			return;
 		}
 
-		if ((mc > BFI_MC_MAX) || (mod->mbhdlr[mc].cbfn == NULL))
+		if ((mc >= BFI_MC_MAX) || (mod->mbhdlr[mc].cbfn == NULL))
 			return;
 
 		mod->mbhdlr[mc].cbfn(mod->mbhdlr[mc].cbarg, &m);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch -resend] [SCSI] bfa: dereferencing freed memory in bfad_im_probe()
       [not found] <20120627085800.GA3007@mwanda>
  2012-06-27  8:59 ` [patch -resend] [SCSI] bfa: off by one in bfa_ioc_mbox_isr() Dan Carpenter
@ 2012-06-27  8:59 ` Dan Carpenter
  2012-06-27 17:45   ` Krishna Gudipati
  2012-06-27  9:00 ` [patch -resend] [SCSI] megaraid: remove a spurious IRQ enable Dan Carpenter
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  8:59 UTC (permalink / raw)
  To: Jing Huang
  Cc: Krishna C Gudipati, James E.J. Bottomley, linux-scsi,
	linux-kernel, kernel-janitors

If bfad_thread_workq(bfad) was not BFA_STATUS_OK then we freed "im"
and then dereferenced it.

I did a little clean up because it seemed nicer to return directly
instead of doing a superfluous goto.  I looked at other functions in
this file and it seems like returning directly is standard.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is the third time I have sent this patch.  It was previously sent
on Fri, 29 Jul 2011 and Wed, 29 Feb 2012.

diff --git a/drivers/scsi/bfa/bfad_im.c b/drivers/scsi/bfa/bfad_im.c
index 1ac09af..2eebf8d 100644
--- a/drivers/scsi/bfa/bfad_im.c
+++ b/drivers/scsi/bfa/bfad_im.c
@@ -687,25 +687,21 @@ bfa_status_t
 bfad_im_probe(struct bfad_s *bfad)
 {
 	struct bfad_im_s      *im;
-	bfa_status_t    rc = BFA_STATUS_OK;
 
 	im = kzalloc(sizeof(struct bfad_im_s), GFP_KERNEL);
-	if (im == NULL) {
-		rc = BFA_STATUS_ENOMEM;
-		goto ext;
-	}
+	if (im == NULL)
+		return BFA_STATUS_ENOMEM;
 
 	bfad->im = im;
 	im->bfad = bfad;
 
 	if (bfad_thread_workq(bfad) != BFA_STATUS_OK) {
 		kfree(im);
-		rc = BFA_STATUS_FAILED;
+		return BFA_STATUS_FAILED;
 	}
 
 	INIT_WORK(&im->aen_im_notify_work, bfad_aen_im_notify_handler);
-ext:
-	return rc;
+	return BFA_STATUS_OK;
 }
 
 void

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch -resend] [SCSI] megaraid: remove a spurious IRQ enable
       [not found] <20120627085800.GA3007@mwanda>
  2012-06-27  8:59 ` [patch -resend] [SCSI] bfa: off by one in bfa_ioc_mbox_isr() Dan Carpenter
  2012-06-27  8:59 ` [patch -resend] [SCSI] bfa: dereferencing freed memory in bfad_im_probe() Dan Carpenter
@ 2012-06-27  9:00 ` Dan Carpenter
  2012-06-27 22:36   ` adam radford
  2012-06-27  9:00 ` [patch 1/2 -resend] SCSI: advansys: handle errors from scsi_dma_map() Dan Carpenter
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:00 UTC (permalink / raw)
  To: Neela Syam Kolli
  Cc: James E.J. Bottomley, linux-scsi, linux-kernel, kernel-janitors,
	Christoph Hellwig

We took this lock with spin_lock() so we should unlock it with
spin_unlock() instead of spin_unlock_irq().  This was introduced in
f2c8dc402b "[SCSI] megaraid_mbox: remove scsi_assign_lock usage".

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This was originally sent on Sat, 30 Jul 2011.  I have cleaned up the
commit message a bit and added Christoph to the CC list.

diff --git a/drivers/scsi/megaraid/megaraid_mbox.c b/drivers/scsi/megaraid/megaraid_mbox.c
index 35bd138..54b1c5b 100644
--- a/drivers/scsi/megaraid/megaraid_mbox.c
+++ b/drivers/scsi/megaraid/megaraid_mbox.c
@@ -2731,7 +2731,7 @@ megaraid_reset_handler(struct scsi_cmnd *scp)
 	}
 
  out:
-	spin_unlock_irq(&adapter->lock);
+	spin_unlock(&adapter->lock);
 	return rval;
 }
 

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch 1/2 -resend] SCSI: advansys: handle errors from scsi_dma_map()
       [not found] <20120627085800.GA3007@mwanda>
                   ` (2 preceding siblings ...)
  2012-06-27  9:00 ` [patch -resend] [SCSI] megaraid: remove a spurious IRQ enable Dan Carpenter
@ 2012-06-27  9:00 ` Dan Carpenter
  2012-06-27 10:01   ` walter harms
  2012-06-27  9:01 ` [patch 2/2 -resend] SCSI: advansys: use a subsystem error code Dan Carpenter
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:00 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: James E.J. Bottomley, linux-scsi, linux-kernel, kernel-janitors

scsi_dma_map() returns -ENOMEM on error.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Originally sent on Tue, 20 Sep 2011.

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 374c4ed..b2c3a1a 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -8426,6 +8426,12 @@ static int asc_build_req(struct asc_board *boardp, struct scsi_cmnd *scp,
 
 	/* Build ASC_SCSI_Q */
 	use_sg = scsi_dma_map(scp);
+	if (use_sg < 0) {
+		scsi_dma_unmap(scp);
+		scp->result = HOST_BYTE(DID_SOFT_ERROR);
+		return ASC_ERROR;
+	}
+
 	if (use_sg != 0) {
 		int sgcnt;
 		struct scatterlist *slp;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch 2/2 -resend] SCSI: advansys: use a subsystem error code
       [not found] <20120627085800.GA3007@mwanda>
                   ` (3 preceding siblings ...)
  2012-06-27  9:00 ` [patch 1/2 -resend] SCSI: advansys: handle errors from scsi_dma_map() Dan Carpenter
@ 2012-06-27  9:01 ` Dan Carpenter
  2012-06-27  9:04 ` [patch 1/3 -resend] [SCSI] pmcraid: remove unneeded check Dan Carpenter
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:01 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: James E.J. Bottomley, linux-scsi, linux-kernel, kernel-janitors

This was supposed to be a subsystem specific error code.  It makes
static checkers complain because "warn_code" is unsigned short so it
can't store negatives.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Originally sent on Tue, 20 Sep 2011.

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index b2c3a1a..4212586 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -4724,7 +4724,7 @@ static ushort AscInitMicroCodeVar(ASC_DVC_VAR *asc_dvc)
 	asc_dvc->overrun_dma = dma_map_single(board->dev, asc_dvc->overrun_buf,
 					ASC_OVERRUN_BSIZE, DMA_FROM_DEVICE);
 	if (dma_mapping_error(board->dev, asc_dvc->overrun_dma)) {
-		warn_code = -ENOMEM;
+		warn_code = UW_ERR;
 		goto err_dma_map;
 	}
 	phy_addr = cpu_to_le32(asc_dvc->overrun_dma);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch 1/3 -resend] [SCSI] pmcraid: remove unneeded check
       [not found] <20120627085800.GA3007@mwanda>
                   ` (4 preceding siblings ...)
  2012-06-27  9:01 ` [patch 2/2 -resend] SCSI: advansys: use a subsystem error code Dan Carpenter
@ 2012-06-27  9:04 ` Dan Carpenter
  2012-06-27  9:04 ` [patch 2/3 -resend] [SCSI] pmcraid: cpu_to_le32() => cpu_to_le64() Dan Carpenter
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:04 UTC (permalink / raw)
  To: Anil Ravindranath
  Cc: James E.J. Bottomley, linux-scsi, linux-kernel, kernel-janitors

request_size is zero here so this condition is always false.  Also we
already handled negative values some lines earlier so it's not negative
for that reason as well.

It looks like this was introduced because we applied Dan Rosenberg's fix
twice by mistake:
b5b515445f4 "[SCSI] pmcraid: reject negative request size"
5f6279da376 "[SCSI] pmcraid: reject negative request size"

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I have tweaked the changelog slightly.  This was originally sent on
Fri, 16 Dec 2011.

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index ea8a0b4..d81a159 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -3870,9 +3870,6 @@ static long pmcraid_ioctl_passthrough(
 			pmcraid_err("couldn't build passthrough ioadls\n");
 			goto out_free_buffer;
 		}
-	} else if (request_size < 0) {
-		rc = -EINVAL;
-		goto out_free_buffer;
 	}
 
 	/* If data is being written into the device, copy the data from user

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch 2/3 -resend] [SCSI] pmcraid: cpu_to_le32() => cpu_to_le64()
       [not found] <20120627085800.GA3007@mwanda>
                   ` (5 preceding siblings ...)
  2012-06-27  9:04 ` [patch 1/3 -resend] [SCSI] pmcraid: remove unneeded check Dan Carpenter
@ 2012-06-27  9:04 ` Dan Carpenter
  2012-06-27  9:04 ` [patch 3/3 -resend] [SCSI] pmcraid: find_first_zero_bit() takes bits not bytes Dan Carpenter
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:04 UTC (permalink / raw)
  To: Anil Ravindranath
  Cc: James E.J. Bottomley, linux-scsi, linux-kernel, kernel-janitors

The cpu_to_le32() truncates the address to 32 bits.  All the other
places that set .address use cpu_to_le64().

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
What about if dma_addr_t is 32 bits on a big endian system?  Can that
happen?

This patch was originally sent on Fri, 16 Dec 2011 and I sent a second
reminder on Mon, Apr 30, 2012.  I was silently ignored both times.

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index d81a159..a38dade 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -1242,7 +1242,7 @@ static struct pmcraid_cmd *pmcraid_init_hcam
 
 	ioadl[0].flags |= IOADL_FLAGS_READ_LAST;
 	ioadl[0].data_len = cpu_to_le32(rcb_size);
-	ioadl[0].address = cpu_to_le32(dma);
+	ioadl[0].address = cpu_to_le64(dma);
 
 	cmd->cmd_done = cmd_done;
 	return cmd;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch 3/3 -resend] [SCSI] pmcraid: find_first_zero_bit() takes bits not bytes
       [not found] <20120627085800.GA3007@mwanda>
                   ` (6 preceding siblings ...)
  2012-06-27  9:04 ` [patch 2/3 -resend] [SCSI] pmcraid: cpu_to_le32() => cpu_to_le64() Dan Carpenter
@ 2012-06-27  9:04 ` Dan Carpenter
  2012-06-27  9:05 ` [patch -resend] [SCSI] isci: add a couple __iomem annotations Dan Carpenter
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:04 UTC (permalink / raw)
  To: Anil Ravindranath
  Cc: James E.J. Bottomley, linux-scsi, linux-kernel, kernel-janitors

The find_first_zero_bit() function takes the size in bits not bytes.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This patch was originally sent on Wed, 2 May 2012.

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index a38dade..719619a 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -5376,7 +5376,7 @@ static unsigned short pmcraid_get_minor(void)
 {
 	int minor;
 
-	minor = find_first_zero_bit(pmcraid_minor, sizeof(pmcraid_minor));
+	minor = find_first_zero_bit(pmcraid_minor, PMCRAID_MAX_ADAPTERS);
 	__set_bit(minor, pmcraid_minor);
 	return minor;
 }

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch -resend] [SCSI] isci: add a couple __iomem annotations
       [not found] <20120627085800.GA3007@mwanda>
                   ` (7 preceding siblings ...)
  2012-06-27  9:04 ` [patch 3/3 -resend] [SCSI] pmcraid: find_first_zero_bit() takes bits not bytes Dan Carpenter
@ 2012-06-27  9:05 ` Dan Carpenter
  2012-06-27 20:58   ` Dan Williams
  2012-06-27  9:05 ` [SCSI] bfa: Implement LUN Masking feature using the SCSI Slave Callouts Dan Carpenter
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:05 UTC (permalink / raw)
  To: Intel SCU Linux support
  Cc: Dan Williams, Dave Jiang, Ed Nadolski, James E.J. Bottomley,
	linux-scsi, linux-kernel, kernel-janitors

These are __iomem.  Sparse complains if we don't have that.

drivers/scsi/isci/phy.c +149 70: warning:
        incorrect type in initializer (different address spaces)

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Sent on Wed, 18 Jan 2012.

diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index bc8981e..4a095f3 100644
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -1973,7 +1973,7 @@ static void sci_controller_afe_initialization(struct isci_host *ihost)
 	}
 
 	for (phy_id = 0; phy_id < SCI_MAX_PHYS; phy_id++) {
-		struct scu_afe_transceiver *xcvr = &afe->scu_afe_xcvr[phy_id];
+		struct scu_afe_transceiver __iomem *xcvr = &afe->scu_afe_xcvr[phy_id];
 		const struct sci_phy_oem_params *oem_phy = &oem->phys[phy_id];
 		int cable_length_long =
 			is_long_cable(phy_id, cable_selection_mask);
diff --git a/drivers/scsi/isci/phy.c b/drivers/scsi/isci/phy.c
index 18f43d4..7b60353 100644
--- a/drivers/scsi/isci/phy.c
+++ b/drivers/scsi/isci/phy.c
@@ -169,7 +169,7 @@ sci_phy_link_layer_initialization(struct isci_phy *iphy,
 	phy_cap.gen1_no_ssc = 1;
 	if (ihost->oem_parameters.controller.do_enable_ssc) {
 		struct scu_afe_registers __iomem *afe = &ihost->scu_registers->afe;
-		struct scu_afe_transceiver *xcvr = &afe->scu_afe_xcvr[phy_idx];
+		struct scu_afe_transceiver __iomem *xcvr = &afe->scu_afe_xcvr[phy_idx];
 		struct isci_pci_info *pci_info = to_pci_info(ihost->pdev);
 		bool en_sas = false;
 		bool en_sata = false;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [SCSI] bfa: Implement LUN Masking feature using the SCSI Slave Callouts.
       [not found] <20120627085800.GA3007@mwanda>
                   ` (8 preceding siblings ...)
  2012-06-27  9:05 ` [patch -resend] [SCSI] isci: add a couple __iomem annotations Dan Carpenter
@ 2012-06-27  9:05 ` Dan Carpenter
  2012-06-27  9:08 ` [patch -resend] [SCSI] megaraid: cleanup type issue in mega_build_cmd() Dan Carpenter
  2012-06-27  9:10 ` [patch -resend] isci: make function declaration match implementation Dan Carpenter
  11 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:05 UTC (permalink / raw)
  To: kgudipat; +Cc: linux-scsi, linux-kernel

Hi,

This bug is still present in linux-next.

regards,
dan carpenter

On Wed, Jan 11, 2012 at 12:32:34PM +0300, Dan Carpenter wrote:
> Hello Krishna Gudipati,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 5b7db7af522d: "[SCSI] bfa: Implement LUN Masking feature 
> using the SCSI Slave Callouts." from Dec 20, 2011, leads to the 
> following Smatch complaint:
> 
> drivers/scsi/bfa/bfad_im.c +962 bfad_im_slave_alloc()
> 	 warn: variable dereferenced before check 'rport' (see line 959)
> 
> drivers/scsi/bfa/bfad_im.c
>    957          struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
>    958		struct bfad_itnim_data_s *itnim_data =
>    959					(struct bfad_itnim_data_s *) rport->dd_data;
>                                                                      ^^^^^^^
> New dereference.
> 
>    960		struct bfa_s *bfa = itnim_data->itnim->bfa_itnim->bfa;
>    961	
>    962		if (!rport || fc_remote_port_chkready(rport))
>                      ^^^^^
> Old check.
> 
>    963			return -ENXIO;
>    964	
> 
> regards,
> dan carpenter
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [patch -resend] [SCSI] megaraid: cleanup type issue in mega_build_cmd()
       [not found] <20120627085800.GA3007@mwanda>
                   ` (9 preceding siblings ...)
  2012-06-27  9:05 ` [SCSI] bfa: Implement LUN Masking feature using the SCSI Slave Callouts Dan Carpenter
@ 2012-06-27  9:08 ` Dan Carpenter
  2012-06-27 22:36   ` adam radford
  2012-06-27  9:10 ` [patch -resend] isci: make function declaration match implementation Dan Carpenter
  11 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:08 UTC (permalink / raw)
  To: Neela Syam Kolli
  Cc: James E.J. Bottomley, linux-scsi, linux-kernel, kernel-janitors

On 64 bit systems the current code sets 32 bits of "seg" and leaves the
other 32 uninitialized.  It doesn't matter since the variable is never
used.  But it's still messy and we should fix it.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Originally sent on Fri, 2 Mar 2012.

diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 4d39a9f..97825f1 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -524,7 +524,7 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int *busy)
 	mega_passthru	*pthru;
 	scb_t	*scb;
 	mbox_t	*mbox;
-	long	seg;
+	u32	seg;
 	char	islogical;
 	int	max_ldrv_num;
 	int	channel = 0;
@@ -858,7 +858,7 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int *busy)
 
 			/* Calculate Scatter-Gather info */
 			mbox->m_out.numsgelements = mega_build_sglist(adapter, scb,
-					(u32 *)&mbox->m_out.xferaddr, (u32 *)&seg);
+					(u32 *)&mbox->m_out.xferaddr, &seg);
 
 			return scb;
 

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [patch -resend] isci: make function declaration match implementation
       [not found] <20120627085800.GA3007@mwanda>
                   ` (10 preceding siblings ...)
  2012-06-27  9:08 ` [patch -resend] [SCSI] megaraid: cleanup type issue in mega_build_cmd() Dan Carpenter
@ 2012-06-27  9:10 ` Dan Carpenter
  11 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27  9:10 UTC (permalink / raw)
  To: Intel SCU Linux support
  Cc: Dan Williams, Dave Jiang, Ed Nadolski, James E.J. Bottomley,
	linux-scsi, linux-kernel, kernel-janitors

Sparse complains that we redeclare this with a different type, because
in the .c file we use an enum and in the .h file we declare the
parameter as a u32.  Probably it's best to use an enum in both places.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I originally sent this on Tue, 27 Mar 2012, and Emil Goode sent the same
patch on Fri, May 4, 2012.  Both were silently ignored.

diff --git a/drivers/scsi/isci/remote_node_context.h b/drivers/scsi/isci/remote_node_context.h
index a703b9c..c7ee81d 100644
--- a/drivers/scsi/isci/remote_node_context.h
+++ b/drivers/scsi/isci/remote_node_context.h
@@ -212,7 +212,7 @@ enum sci_status sci_remote_node_context_destruct(struct sci_remote_node_context
 						      scics_sds_remote_node_context_callback callback,
 						      void *callback_parameter);
 enum sci_status sci_remote_node_context_suspend(struct sci_remote_node_context *sci_rnc,
-						     u32 suspend_type,
+						     enum sci_remote_node_suspension_reasons reason,
 						     u32 suspension_code);
 enum sci_status sci_remote_node_context_resume(struct sci_remote_node_context *sci_rnc,
 						    scics_sds_remote_node_context_callback cb_fn,

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [patch 1/2 -resend] SCSI: advansys: handle errors from scsi_dma_map()
  2012-06-27  9:00 ` [patch 1/2 -resend] SCSI: advansys: handle errors from scsi_dma_map() Dan Carpenter
@ 2012-06-27 10:01   ` walter harms
  2012-06-27 10:15     ` Dan Carpenter
  0 siblings, 1 reply; 19+ messages in thread
From: walter harms @ 2012-06-27 10:01 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Matthew Wilcox, James E.J. Bottomley, linux-scsi, kernel-janitors



Am 27.06.2012 11:00, schrieb Dan Carpenter:
> scsi_dma_map() returns -ENOMEM on error.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Originally sent on Tue, 20 Sep 2011.
> 
> diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
> index 374c4ed..b2c3a1a 100644
> --- a/drivers/scsi/advansys.c
> +++ b/drivers/scsi/advansys.c
> @@ -8426,6 +8426,12 @@ static int asc_build_req(struct asc_board *boardp, struct scsi_cmnd *scp,
>  
>  	/* Build ASC_SCSI_Q */
>  	use_sg = scsi_dma_map(scp);
> +	if (use_sg < 0) {
> +		scsi_dma_unmap(scp);
> +		scp->result = HOST_BYTE(DID_SOFT_ERROR);
> +		return ASC_ERROR;
> +	}
> +
>  	if (use_sg != 0) {
>  		int sgcnt;
>  		struct scatterlist *slp;

Q:if use_sg == 0 a special case ?

re,
 wh

> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [patch 1/2 -resend] SCSI: advansys: handle errors from scsi_dma_map()
  2012-06-27 10:01   ` walter harms
@ 2012-06-27 10:15     ` Dan Carpenter
  0 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-06-27 10:15 UTC (permalink / raw)
  To: walter harms
  Cc: Matthew Wilcox, James E.J. Bottomley, linux-scsi, kernel-janitors

On Wed, Jun 27, 2012 at 12:01:01PM +0200, walter harms wrote:
> 
> 
> Am 27.06.2012 11:00, schrieb Dan Carpenter:
> > scsi_dma_map() returns -ENOMEM on error.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > Originally sent on Tue, 20 Sep 2011.
> > 
> > diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
> > index 374c4ed..b2c3a1a 100644
> > --- a/drivers/scsi/advansys.c
> > +++ b/drivers/scsi/advansys.c
> > @@ -8426,6 +8426,12 @@ static int asc_build_req(struct asc_board *boardp, struct scsi_cmnd *scp,
> >  
> >  	/* Build ASC_SCSI_Q */
> >  	use_sg = scsi_dma_map(scp);
> > +	if (use_sg < 0) {
> > +		scsi_dma_unmap(scp);
> > +		scp->result = HOST_BYTE(DID_SOFT_ERROR);
> > +		return ASC_ERROR;
> > +	}
> > +
> >  	if (use_sg != 0) {
> >  		int sgcnt;
> >  		struct scatterlist *slp;
> 
> Q:if use_sg == 0 a special case ?
> 

Yes.  scsi_dma_map() returns the number of sg lists actually used,
zero if the sg lists is NULL, or -ENOMEM if the mapping failed.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [patch -resend] [SCSI] bfa: off by one in bfa_ioc_mbox_isr()
  2012-06-27  8:59 ` [patch -resend] [SCSI] bfa: off by one in bfa_ioc_mbox_isr() Dan Carpenter
@ 2012-06-27 17:44   ` Krishna Gudipati
  0 siblings, 0 replies; 19+ messages in thread
From: Krishna Gudipati @ 2012-06-27 17:44 UTC (permalink / raw)
  To: 'Dan Carpenter', Jing Huang
  Cc: 'James E.J. Bottomley',
	'linux-scsi@vger.kernel.org',
	'linux-kernel@vger.kernel.org',
	'kernel-janitors@vger.kernel.org'

-----Original Message-----
From: Dan Carpenter [mailto:dan.carpenter@oracle.com] 
Sent: Wednesday, June 27, 2012 2:00 AM
To: Jing Huang
Cc: Krishna Gudipati; James E.J. Bottomley; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
Subject: [patch -resend] [SCSI] bfa: off by one in bfa_ioc_mbox_isr()

If mc == BFI_MC_MAX then we're reading past the end of the
mod->mbhdlr[] array.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Originally sent on Wed, 6 Jul 2011.

diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index 14e6284..8cdb79c 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -2357,7 +2357,7 @@ bfa_ioc_mbox_isr(struct bfa_ioc_s *ioc)
 			return;
 		}
 
-		if ((mc > BFI_MC_MAX) || (mod->mbhdlr[mc].cbfn == NULL))
+		if ((mc >= BFI_MC_MAX) || (mod->mbhdlr[mc].cbfn == NULL))
 			return;
 
 		mod->mbhdlr[mc].cbfn(mod->mbhdlr[mc].cbarg, &m);


-----

Thanks for the patch.

Acked-by: Krishna Gudipati <kgudipat@brocade.com>

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* RE: [patch -resend] [SCSI] bfa: dereferencing freed memory in bfad_im_probe()
  2012-06-27  8:59 ` [patch -resend] [SCSI] bfa: dereferencing freed memory in bfad_im_probe() Dan Carpenter
@ 2012-06-27 17:45   ` Krishna Gudipati
  0 siblings, 0 replies; 19+ messages in thread
From: Krishna Gudipati @ 2012-06-27 17:45 UTC (permalink / raw)
  To: 'Dan Carpenter', Jing Huang
  Cc: 'James E.J. Bottomley',
	'linux-scsi@vger.kernel.org',
	'linux-kernel@vger.kernel.org',
	'kernel-janitors@vger.kernel.org'



-----Original Message-----
From: Dan Carpenter [mailto:dan.carpenter@oracle.com] 
Sent: Wednesday, June 27, 2012 2:00 AM
To: Jing Huang
Cc: Krishna Gudipati; James E.J. Bottomley; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
Subject: [patch -resend] [SCSI] bfa: dereferencing freed memory in bfad_im_probe()

If bfad_thread_workq(bfad) was not BFA_STATUS_OK then we freed "im"
and then dereferenced it.

I did a little clean up because it seemed nicer to return directly
instead of doing a superfluous goto.  I looked at other functions in
this file and it seems like returning directly is standard.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is the third time I have sent this patch.  It was previously sent
on Fri, 29 Jul 2011 and Wed, 29 Feb 2012.

diff --git a/drivers/scsi/bfa/bfad_im.c b/drivers/scsi/bfa/bfad_im.c
index 1ac09af..2eebf8d 100644
--- a/drivers/scsi/bfa/bfad_im.c
+++ b/drivers/scsi/bfa/bfad_im.c
@@ -687,25 +687,21 @@ bfa_status_t
 bfad_im_probe(struct bfad_s *bfad)
 {
 	struct bfad_im_s      *im;
-	bfa_status_t    rc = BFA_STATUS_OK;
 
 	im = kzalloc(sizeof(struct bfad_im_s), GFP_KERNEL);
-	if (im == NULL) {
-		rc = BFA_STATUS_ENOMEM;
-		goto ext;
-	}
+	if (im == NULL)
+		return BFA_STATUS_ENOMEM;
 
 	bfad->im = im;
 	im->bfad = bfad;
 
 	if (bfad_thread_workq(bfad) != BFA_STATUS_OK) {
 		kfree(im);
-		rc = BFA_STATUS_FAILED;
+		return BFA_STATUS_FAILED;
 	}
 
 	INIT_WORK(&im->aen_im_notify_work, bfad_aen_im_notify_handler);
-ext:
-	return rc;
+	return BFA_STATUS_OK;
 }
 
 void

-----

Thanks for the patch.

Acked-by: Krishna Gudipati <kgudipat@brocade.com>

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [patch -resend] [SCSI] isci: add a couple __iomem annotations
  2012-06-27  9:05 ` [patch -resend] [SCSI] isci: add a couple __iomem annotations Dan Carpenter
@ 2012-06-27 20:58   ` Dan Williams
  0 siblings, 0 replies; 19+ messages in thread
From: Dan Williams @ 2012-06-27 20:58 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Intel SCU Linux support, Dave Jiang, Ed Nadolski,
	James E.J. Bottomley, linux-scsi, linux-kernel, kernel-janitors

On Wed, Jun 27, 2012 at 2:05 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> These are __iomem.  Sparse complains if we don't have that.
>
> drivers/scsi/isci/phy.c +149 70: warning:
>        incorrect type in initializer (different address spaces)
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Sent on Wed, 18 Jan 2012.
>

Thanks for nudge, this fell off the radar.  I'll get this into -next
with the rest of the pending bits.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [patch -resend] [SCSI] megaraid: cleanup type issue in mega_build_cmd()
  2012-06-27  9:08 ` [patch -resend] [SCSI] megaraid: cleanup type issue in mega_build_cmd() Dan Carpenter
@ 2012-06-27 22:36   ` adam radford
  0 siblings, 0 replies; 19+ messages in thread
From: adam radford @ 2012-06-27 22:36 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Neela Syam Kolli, James E.J. Bottomley, linux-scsi, linux-kernel,
	kernel-janitors

On Wed, Jun 27, 2012 at 2:08 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On 64 bit systems the current code sets 32 bits of "seg" and leaves the
> other 32 uninitialized.  It doesn't matter since the variable is never
> used.  But it's still messy and we should fix it.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Originally sent on Fri, 2 Mar 2012.
>
> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
> index 4d39a9f..97825f1 100644
> --- a/drivers/scsi/megaraid.c
> +++ b/drivers/scsi/megaraid.c
> @@ -524,7 +524,7 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int *busy)
>        mega_passthru   *pthru;
>        scb_t   *scb;
>        mbox_t  *mbox;
> -       long    seg;
> +       u32     seg;
>        char    islogical;
>        int     max_ldrv_num;
>        int     channel = 0;
> @@ -858,7 +858,7 @@ mega_build_cmd(adapter_t *adapter, Scsi_Cmnd *cmd, int *busy)
>
>                        /* Calculate Scatter-Gather info */
>                        mbox->m_out.numsgelements = mega_build_sglist(adapter, scb,
> -                                       (u32 *)&mbox->m_out.xferaddr, (u32 *)&seg);
> +                                       (u32 *)&mbox->m_out.xferaddr, &seg);
>
>                        return scb;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Acked-by: Adam Radford <aradford@gmail.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [patch -resend] [SCSI] megaraid: remove a spurious IRQ enable
  2012-06-27  9:00 ` [patch -resend] [SCSI] megaraid: remove a spurious IRQ enable Dan Carpenter
@ 2012-06-27 22:36   ` adam radford
  0 siblings, 0 replies; 19+ messages in thread
From: adam radford @ 2012-06-27 22:36 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Neela Syam Kolli, James E.J. Bottomley, linux-scsi, linux-kernel,
	kernel-janitors, Christoph Hellwig

On Wed, Jun 27, 2012 at 2:00 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> We took this lock with spin_lock() so we should unlock it with
> spin_unlock() instead of spin_unlock_irq().  This was introduced in
> f2c8dc402b "[SCSI] megaraid_mbox: remove scsi_assign_lock usage".
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> This was originally sent on Sat, 30 Jul 2011.  I have cleaned up the
> commit message a bit and added Christoph to the CC list.
>
> diff --git a/drivers/scsi/megaraid/megaraid_mbox.c b/drivers/scsi/megaraid/megaraid_mbox.c
> index 35bd138..54b1c5b 100644
> --- a/drivers/scsi/megaraid/megaraid_mbox.c
> +++ b/drivers/scsi/megaraid/megaraid_mbox.c
> @@ -2731,7 +2731,7 @@ megaraid_reset_handler(struct scsi_cmnd *scp)
>        }
>
>  out:
> -       spin_unlock_irq(&adapter->lock);
> +       spin_unlock(&adapter->lock);
>        return rval;
>  }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Acked-by: Adam Radford <aradford@gmail.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2012-06-27 22:36 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20120627085800.GA3007@mwanda>
2012-06-27  8:59 ` [patch -resend] [SCSI] bfa: off by one in bfa_ioc_mbox_isr() Dan Carpenter
2012-06-27 17:44   ` Krishna Gudipati
2012-06-27  8:59 ` [patch -resend] [SCSI] bfa: dereferencing freed memory in bfad_im_probe() Dan Carpenter
2012-06-27 17:45   ` Krishna Gudipati
2012-06-27  9:00 ` [patch -resend] [SCSI] megaraid: remove a spurious IRQ enable Dan Carpenter
2012-06-27 22:36   ` adam radford
2012-06-27  9:00 ` [patch 1/2 -resend] SCSI: advansys: handle errors from scsi_dma_map() Dan Carpenter
2012-06-27 10:01   ` walter harms
2012-06-27 10:15     ` Dan Carpenter
2012-06-27  9:01 ` [patch 2/2 -resend] SCSI: advansys: use a subsystem error code Dan Carpenter
2012-06-27  9:04 ` [patch 1/3 -resend] [SCSI] pmcraid: remove unneeded check Dan Carpenter
2012-06-27  9:04 ` [patch 2/3 -resend] [SCSI] pmcraid: cpu_to_le32() => cpu_to_le64() Dan Carpenter
2012-06-27  9:04 ` [patch 3/3 -resend] [SCSI] pmcraid: find_first_zero_bit() takes bits not bytes Dan Carpenter
2012-06-27  9:05 ` [patch -resend] [SCSI] isci: add a couple __iomem annotations Dan Carpenter
2012-06-27 20:58   ` Dan Williams
2012-06-27  9:05 ` [SCSI] bfa: Implement LUN Masking feature using the SCSI Slave Callouts Dan Carpenter
2012-06-27  9:08 ` [patch -resend] [SCSI] megaraid: cleanup type issue in mega_build_cmd() Dan Carpenter
2012-06-27 22:36   ` adam radford
2012-06-27  9:10 ` [patch -resend] isci: make function declaration match implementation Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox