Patchwork pseries: Fix array overrun bug in PCI code

login
register
mail settings
Submitter David Gibson
Date Nov. 29, 2011, 6:21 a.m.
Message ID <1322547699-28329-1-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/128220/
State New
Headers show

Comments

David Gibson - Nov. 29, 2011, 6:21 a.m.
spapr_populate_pci_devices() containd a loop with PCI_NUM_REGIONS (7)
iterations.  However this overruns the 'bars' global array, which only has
6 elements. In fact we only want to run this loop for things listed in the
bars array, so this patch corrects the loop bounds to reflect that.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/spapr_pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
David Gibson - Nov. 29, 2011, 7:41 a.m.
On Tue, Nov 29, 2011 at 05:21:39PM +1100, David Gibson wrote:
> spapr_populate_pci_devices() containd a loop with PCI_NUM_REGIONS (7)
> iterations.  However this overruns the 'bars' global array, which only has
> 6 elements. In fact we only want to run this loop for things listed in the
> bars array, so this patch corrects the loop bounds to reflect that.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

As a bugfix for a bad memory access, this is definitely for 1.0
andrzej zaborowski - Dec. 5, 2011, 8:24 p.m.
On 29 November 2011 08:41, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Tue, Nov 29, 2011 at 05:21:39PM +1100, David Gibson wrote:
>> spapr_populate_pci_devices() containd a loop with PCI_NUM_REGIONS (7)
>> iterations.  However this overruns the 'bars' global array, which only has
>> 6 elements. In fact we only want to run this loop for things listed in the
>> bars array, so this patch corrects the loop bounds to reflect that.
>>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> As a bugfix for a bad memory access, this is definitely for 1.0

Now applied.  Indeed it would probably have been better done before the release.

Cheers
Alexander Graf - Dec. 5, 2011, 8:33 p.m.
On 05.12.2011, at 21:24, andrzej zaborowski wrote:

> On 29 November 2011 08:41, David Gibson <david@gibson.dropbear.id.au> wrote:
>> On Tue, Nov 29, 2011 at 05:21:39PM +1100, David Gibson wrote:
>>> spapr_populate_pci_devices() containd a loop with PCI_NUM_REGIONS (7)
>>> iterations.  However this overruns the 'bars' global array, which only has
>>> 6 elements. In fact we only want to run this loop for things listed in the
>>> bars array, so this patch corrects the loop bounds to reflect that.
>>> 
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> 
>> As a bugfix for a bad memory access, this is definitely for 1.0
> 
> Now applied.  Indeed it would probably have been better done before the release.

Well, that's why we aligned to have 1.0.1 around christmas - because there will be more issues exposed soon, I'm fairly sure :)

Thanks for applying btw.


Alex
Alexander Graf - Dec. 19, 2011, 1:59 p.m.
On 29.11.2011, at 07:21, David Gibson wrote:

> spapr_populate_pci_devices() containd a loop with PCI_NUM_REGIONS (7)
> iterations.  However this overruns the 'bars' global array, which only has
> 6 elements. In fact we only want to run this loop for things listed in the
> bars array, so this patch corrects the loop bounds to reflect that.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Thanks, applied to ppc-next and queued for 1.0.

Alex

> ---
> hw/spapr_pci.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
> index 7162588..9b6a032 100644
> --- a/hw/spapr_pci.c
> +++ b/hw/spapr_pci.c
> @@ -454,7 +454,7 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb,
>         reg[0].size = 0;
> 
>         n = 0;
> -        for (i = 0; i < PCI_NUM_REGIONS; ++i) {
> +        for (i = 0; i < ARRAY_SIZE(bars); ++i) {
>             if (0 == dev->io_regions[i].size) {
>                 continue;
>             }
> -- 
> 1.7.7.3
>
Alexander Graf - Dec. 19, 2011, 2:01 p.m.
On 19.12.2011, at 14:59, Alexander Graf wrote:

> 
> On 29.11.2011, at 07:21, David Gibson wrote:
> 
>> spapr_populate_pci_devices() containd a loop with PCI_NUM_REGIONS (7)
>> iterations.  However this overruns the 'bars' global array, which only has
>> 6 elements. In fact we only want to run this loop for things listed in the
>> bars array, so this patch corrects the loop bounds to reflect that.
>> 
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Thanks, applied to ppc-next and queued for 1.0.

Call me stupid. Only queued for 1.0 of course.


Alex

Patch

diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index 7162588..9b6a032 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -454,7 +454,7 @@  int spapr_populate_pci_devices(sPAPRPHBState *phb,
         reg[0].size = 0;
 
         n = 0;
-        for (i = 0; i < PCI_NUM_REGIONS; ++i) {
+        for (i = 0; i < ARRAY_SIZE(bars); ++i) {
             if (0 == dev->io_regions[i].size) {
                 continue;
             }