2012-05-12 11:00:06

by Dan Carpenter

Subject: [patch] iommu: off by one in dmar_get_fault_reason()

fault_reason == ARRAY_SIZE(irq_remap_fault_reasons) + 0x20 - 0x20 is one
past the end of the array.

Signed-off-by: Dan Carpenter <[email protected]>

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 5ef65cf..9390f3f 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1057,7 +1057,7 @@ static const char *irq_remap_fault_reasons[] =

const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
{
- if (fault_reason >= 0x20 && (fault_reason <= 0x20 +
+ if (fault_reason >= 0x20 && (fault_reason < 0x20 +
ARRAY_SIZE(irq_remap_fault_reasons))) {
*fault_type = INTR_REMAP;
return irq_remap_fault_reasons[fault_reason - 0x20];

2012-05-12 12:37:12

by walter harms

Subject: Re: [patch] iommu: off by one in dmar_get_fault_reason()

Am 12.05.2012 12:59, schrieb Dan Carpenter:
> fault_reason == ARRAY_SIZE(irq_remap_fault_reasons) + 0x20 - 0x20 is one
> past the end of the array.
>
> Signed-off-by: Dan Carpenter <[email protected]>
>
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index 5ef65cf..9390f3f 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -1057,7 +1057,7 @@ static const char *irq_remap_fault_reasons[] =
>
> const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
> {
> - if (fault_reason >= 0x20 && (fault_reason <= 0x20 +
> + if (fault_reason >= 0x20 && (fault_reason < 0x20 +
> ARRAY_SIZE(irq_remap_fault_reasons))) {
> *fault_type = INTR_REMAP;
> return irq_remap_fault_reasons[fault_reason - 0x20];

perhaps this is more readable:
if (fault_reason >= 0x20 && (fault_reason - 0x20 < ARRAY_SIZE(irq_remap_fault_reasons)))

re,
wh

2012-05-13 17:09:10

by Dan Carpenter

Subject: Re: [patch] iommu: off by one in dmar_get_fault_reason()

On Sat, May 12, 2012 at 02:36:42PM +0200, walter harms wrote:
>
>
> Am 12.05.2012 12:59, schrieb Dan Carpenter:
> > fault_reason == ARRAY_SIZE(irq_remap_fault_reasons) + 0x20 - 0x20 is one
> > past the end of the array.
> >
> > Signed-off-by: Dan Carpenter <[email protected]>
> >
> > diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> > index 5ef65cf..9390f3f 100644
> > --- a/drivers/iommu/dmar.c
> > +++ b/drivers/iommu/dmar.c
> > @@ -1057,7 +1057,7 @@ static const char *irq_remap_fault_reasons[] =
> >
> > const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
> > {
> > - if (fault_reason >= 0x20 && (fault_reason <= 0x20 +
> > + if (fault_reason >= 0x20 && (fault_reason < 0x20 +
> > ARRAY_SIZE(irq_remap_fault_reasons))) {
> > *fault_type = INTR_REMAP;
> > return irq_remap_fault_reasons[fault_reason - 0x20];
>
> perhaps this is more readable:
> if (fault_reason >= 0x20 && (fault_reason - 0x20 < ARRAY_SIZE(irq_remap_fault_reasons)))
>

Yeah. Probably you're way is nicer.

regards,
dan carpenter

2012-05-13 17:10:07

by Dan Carpenter

Subject: [patch v2] iommu: off by one in dmar_get_fault_reason()

fault_reason - 0x20 == ARRAY_SIZE(irq_remap_fault_reasons) is one past
the end of the array.

Signed-off-by: Dan Carpenter <[email protected]>
---
v2: re-arranged slightly for readability

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 5ef65cf..3a74e44 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1057,8 +1057,8 @@ static const char *irq_remap_fault_reasons[] =

const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
{
- if (fault_reason >= 0x20 && (fault_reason <= 0x20 +
- ARRAY_SIZE(irq_remap_fault_reasons))) {
+ if (fault_reason >= 0x20 && (fault_reason - 0x20 <
+ ARRAY_SIZE(irq_remap_fault_reasons))) {
*fault_type = INTR_REMAP;
return irq_remap_fault_reasons[fault_reason - 0x20];
} else if (fault_reason < ARRAY_SIZE(dma_remap_fault_reasons)) {

2012-05-14 13:42:27

by Dan Carpenter

Subject: [tip:core/iommu] iommu: Fix off by one in dmar_get_fault_reason()

Commit-ID: fefe1ed1398b81e3fadc92d11d91162d343c8836
Gitweb: http://git.kernel.org/tip/fefe1ed1398b81e3fadc92d11d91162d343c8836
Author: Dan Carpenter <[email protected]>
AuthorDate: Sun, 13 May 2012 20:09:38 +0300
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 14 May 2012 14:37:47 +0200

iommu: Fix off by one in dmar_get_fault_reason()

fault_reason - 0x20 == ARRAY_SIZE(irq_remap_fault_reasons) is
one past the end of the array.

Signed-off-by: Dan Carpenter <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Youquan Song <[email protected]>
Cc: walter harms <[email protected]>
Cc: Suresh Siddha <[email protected]>
Cc: <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/iommu/dmar.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 5ef65cf..3a74e44 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1057,8 +1057,8 @@ static const char *irq_remap_fault_reasons[] =

const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
{
- if (fault_reason >= 0x20 && (fault_reason <= 0x20 +
- ARRAY_SIZE(irq_remap_fault_reasons))) {
+ if (fault_reason >= 0x20 && (fault_reason - 0x20 <
+ ARRAY_SIZE(irq_remap_fault_reasons))) {
*fault_type = INTR_REMAP;
return irq_remap_fault_reasons[fault_reason - 0x20];
} else if (fault_reason < ARRAY_SIZE(dma_remap_fault_reasons)) {