hpsa: fix uninitialized trans_support in hpsa_put_ctlr_into_performant_mode()
Subsystems (3)
PATCH 1/1 hpsa: fix uninitialized trans_support in hpsa_put_ctlr_into_performant_mode()
Without this, you'll see a null pointer dereference in hpsa_enter_performant_mode().
--- drivers/scsi/hpsa.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/scsi/hpsa.c +4 -0
@@ -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 |
Signature · 1 line
1.7.1
Quoted 2 lines · On Thu, 2014-04-10 at 17:17 -0500, [email protected] 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.
This patch works for me.
Thanks
Baoquan
Quoted 27 lines · On 04/10/14 at 05:17pm, [email protected].…
Without this, you'll see a null pointer dereference in
hpsa_enter_performant_mode().Signed-off-by Stephen M. Cameron <[email protected]>--- drivers/scsi/hpsa.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)drivers/scsi/hpsa.c +4
-0@@ -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 |Signature · 1 line
1.7.1
James Bottomley
maintainerYour 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
Quoted 2 lines · On Thu, 2014-04-10 at 17:17 -0500, scameron-3C9H9nn4BS4HL6m8NFMY+dBPR1lH4CV8@pu…
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.
Quoted 10 lines
Signed-off-by Stephen M. Cameron <[email protected]>--- drivers/scsi/hpsa.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)drivers/scsi/hpsa.c +0
-0
drivers/scsi/hpsa.c static void hpsa_put_ctlr_into_performant_mode(struct ctlr_info *h) · line 7472
Quoted 10 lines
drivers/scsi/hpsa.c +4
-0@@ -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
---
drivers/scsi/hpsa.c +4 -4
@@ -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 */
Quoted 27 lines · 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
Quoted 2 lines · On Thu, 2014-04-10 at 17:17 -0500, scameron-3C9H9nn4BS4HL6m8NFMY+dBPR1lH4CV8@pu…
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.
Quoted 13 lines
Signed-off-by Stephen M. Cameron <[email protected]>--- drivers/scsi/hpsa.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)drivers/scsi/hpsa.c +0
-0@@ -7463,6 +7463,10 @@ static void hpsa_put_ctlr_into_performant_mode(struct ctlr_info *h)if (hpsa_simple_mode)return ;
Quoted 10 lines
drivers/scsi/hpsa.c static void hpsa_put_ctlr_into_performant_mode(struct ctlr_info *h) · line 7472
Quoted 7 lines
drivers/scsi/hpsa.c +4
-0+ 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?
Oh... I didn't notice that. So that's what happened to that hunk.
Yes, that is what should be done.
I will resend the patch after fixing up this and the commit message.
-- steve
Quoted 32 lines
James
---
drivers/scsi/hpsa.c +4
-4@@ -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 */