Re: [PATCH] hpsa: fix uninitialized trans_support in hpsa_put_ctlr_into_performant_mode()

2014-04-14 Thread James Bottomley 
Your subject line is very tame.  It should be the one line summary of
why we apply the patch, so it should read something like

 hpsa: fix NULL deref in performant mode

On Thu, 2014-04-10 at 17:17 -0500, scame...@beardog.cce.hp.com wrote:
 Without this, you'll see a null pointer dereference in
 hpsa_enter_performant_mode().

The description should be more comprehensible.

I'm clear that the use before initialisation is a bug ... I'm less clear
on why it causes an oops.

 Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
 ---
  drivers/scsi/hpsa.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
 index 8cf4a0c..ef4dfdd 100644
 --- a/drivers/scsi/hpsa.c
 +++ b/drivers/scsi/hpsa.c
 @@ -7463,6 +7463,10 @@ static void hpsa_put_ctlr_into_performant_mode(struct 
 ctlr_info *h)
   if (hpsa_simple_mode)
   return;
  
 + trans_support = readl((h-cfgtable-TransportSupport));
 + if (!(trans_support  PERFORMANT_MODE))
 + return;
 +
   /* Check for I/O accelerator mode support */
   if (trans_support  CFGTBL_Trans_io_accel1) {
   transMethod |= CFGTBL_Trans_io_accel1 |

Shouldn't you be moving this check from its previous location, rather
than adding a new one that makes the original obsolete?

James

---

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 8cf4a0c..9a6e4a2 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -7463,6 +7463,10 @@ static void hpsa_put_ctlr_into_performant_mode(struct 
ctlr_info *h)
if (hpsa_simple_mode)
return;
 
+   trans_support = readl((h-cfgtable-TransportSupport));
+   if (!(trans_support  PERFORMANT_MODE))
+   return;
+
/* Check for I/O accelerator mode support */
if (trans_support  CFGTBL_Trans_io_accel1) {
transMethod |= CFGTBL_Trans_io_accel1 |
@@ -7479,10 +7483,6 @@ static void hpsa_put_ctlr_into_performant_mode(struct 
ctlr_info *h)
}
 
/* TODO, check that this next line h-nreply_queues is correct */
-   trans_support = readl((h-cfgtable-TransportSupport));
-   if (!(trans_support  PERFORMANT_MODE))
-   return;
-
h-nreply_queues = h-msix_vector  0 ? h-msix_vector : 1;
hpsa_get_max_perf_mode_cmds(h);
/* Performant mode ring buffer and supporting data structures */


--
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] hpsa: fix uninitialized trans_support in hpsa_put_ctlr_into_performant_mode()

2014-04-10 Thread scameron 

Without this, you'll see a null pointer dereference in
hpsa_enter_performant_mode().

Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
---
 drivers/scsi/hpsa.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 8cf4a0c..ef4dfdd 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -7463,6 +7463,10 @@ static void hpsa_put_ctlr_into_performant_mode(struct 
ctlr_info *h)
if (hpsa_simple_mode)
return;
 
+   trans_support = readl((h-cfgtable-TransportSupport));
+   if (!(trans_support  PERFORMANT_MODE))
+   return;
+
/* Check for I/O accelerator mode support */
if (trans_support  CFGTBL_Trans_io_accel1) {
transMethod |= CFGTBL_Trans_io_accel1 |
-- 
1.7.1

--
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] hpsa: fix uninitialized trans_support in hpsa_put_ctlr_into_performant_mode()

2014-04-10 Thread Davidlohr Bueso 
On Thu, 2014-04-10 at 17:17 -0500, scame...@beardog.cce.hp.com wrote:
 Without this, you'll see a null pointer dereference in
 hpsa_enter_performant_mode().

So I'm not surprised that this patch doesn't solve the problem I am
seeing with DMAR and the hpsa driver hard lockup.

In any case it should address Baoquan's original report, so it confirms
that it is in fact two different sets of issues.

--
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] hpsa: fix uninitialized trans_support in hpsa_put_ctlr_into_performant_mode()

2014-04-10 Thread Baoquan He 
This patch works for me.

Tested-by: Baoquan He b...@redhat.com

Thanks
Baoquan

On 04/10/14 at 05:17pm, scame...@beardog.cce.hp.com wrote:
 
 Without this, you'll see a null pointer dereference in
 hpsa_enter_performant_mode().
 
 Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
 ---
  drivers/scsi/hpsa.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
 index 8cf4a0c..ef4dfdd 100644
 --- a/drivers/scsi/hpsa.c
 +++ b/drivers/scsi/hpsa.c
 @@ -7463,6 +7463,10 @@ static void hpsa_put_ctlr_into_performant_mode(struct 
 ctlr_info *h)
   if (hpsa_simple_mode)
   return;
  
 + trans_support = readl((h-cfgtable-TransportSupport));
 + if (!(trans_support  PERFORMANT_MODE))
 + return;
 +
   /* Check for I/O accelerator mode support */
   if (trans_support  CFGTBL_Trans_io_accel1) {
   transMethod |= CFGTBL_Trans_io_accel1 |
 -- 
 1.7.1
 
--
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