Message ID | 1422689277-16032-3-git-send-email-arei.gonglei@huawei.com |
---|---|
State | New |
Headers | show |
On Sat, 31 Jan 2015, arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > The array length of s->real_device.io_regions[] is > "PCI_NUM_REGIONS - 1". Add a check, just make Coverity happy. > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > hw/xen/xen_pt_config_init.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c > index 710fe50..3c8b0f1 100644 > --- a/hw/xen/xen_pt_config_init.c > +++ b/hw/xen/xen_pt_config_init.c > @@ -443,6 +443,11 @@ static int xen_pt_bar_reg_read(XenPCIPassthroughState *s, XenPTReg *cfg_entry, > return -1; > } > > + if (index == PCI_ROM_SLOT) { > + XEN_PT_ERR(&s->dev, "Internal error: Access violation at ROM BAR.\n"); > + return -1; > + } Could you please fix the boundaries of the check just above? Also please avoid using PCI_ROM_SLOT for the array index check, simply use PCI_NUM_REGIONS. > /* use fixed-up value from kernel sysfs */ > *value = base_address_with_flags(&s->real_device.io_regions[index]); > > -- > 1.7.12.4 > >
On 2015/2/10 14:39, Stefano Stabellini wrote: > On Sat, 31 Jan 2015, arei.gonglei@huawei.com wrote: >> From: Gonglei <arei.gonglei@huawei.com> >> >> The array length of s->real_device.io_regions[] is >> "PCI_NUM_REGIONS - 1". Add a check, just make Coverity happy. >> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> --- >> hw/xen/xen_pt_config_init.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c >> index 710fe50..3c8b0f1 100644 >> --- a/hw/xen/xen_pt_config_init.c >> +++ b/hw/xen/xen_pt_config_init.c >> @@ -443,6 +443,11 @@ static int xen_pt_bar_reg_read(XenPCIPassthroughState *s, XenPTReg *cfg_entry, >> return -1; >> } >> >> + if (index == PCI_ROM_SLOT) { >> + XEN_PT_ERR(&s->dev, "Internal error: Access violation at ROM BAR.\n"); >> + return -1; >> + } > > Could you please fix the boundaries of the check just above? > Also please avoid using PCI_ROM_SLOT for the array index check, simply > use PCI_NUM_REGIONS. > You meaning is changing the below check: if (index < 0 || index >= PCI_NUM_REGIONS - 1) { XEN_PT_ERR(&s->dev, "Internal error: Invalid BAR index [%d].\n", index); return -1; } Isn't it? Regards, -Gonglei
On Tue, 10 Feb 2015, Gonglei wrote: > On 2015/2/10 14:39, Stefano Stabellini wrote: > > On Sat, 31 Jan 2015, arei.gonglei@huawei.com wrote: > >> From: Gonglei <arei.gonglei@huawei.com> > >> > >> The array length of s->real_device.io_regions[] is > >> "PCI_NUM_REGIONS - 1". Add a check, just make Coverity happy. > >> > >> Signed-off-by: Gonglei <arei.gonglei@huawei.com> > >> --- > >> hw/xen/xen_pt_config_init.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c > >> index 710fe50..3c8b0f1 100644 > >> --- a/hw/xen/xen_pt_config_init.c > >> +++ b/hw/xen/xen_pt_config_init.c > >> @@ -443,6 +443,11 @@ static int xen_pt_bar_reg_read(XenPCIPassthroughState *s, XenPTReg *cfg_entry, > >> return -1; > >> } > >> > >> + if (index == PCI_ROM_SLOT) { > >> + XEN_PT_ERR(&s->dev, "Internal error: Access violation at ROM BAR.\n"); > >> + return -1; > >> + } > > > > Could you please fix the boundaries of the check just above? > > Also please avoid using PCI_ROM_SLOT for the array index check, simply > > use PCI_NUM_REGIONS. > > > You meaning is changing the below check: > > if (index < 0 || index >= PCI_NUM_REGIONS - 1) { > XEN_PT_ERR(&s->dev, "Internal error: Invalid BAR index [%d].\n", index); > return -1; > } > > Isn't it? that's right
On 2015/2/10 15:00, Stefano Stabellini wrote: > On Tue, 10 Feb 2015, Gonglei wrote: >> On 2015/2/10 14:39, Stefano Stabellini wrote: >>> On Sat, 31 Jan 2015, arei.gonglei@huawei.com wrote: >>>> From: Gonglei <arei.gonglei@huawei.com> >>>> >>>> The array length of s->real_device.io_regions[] is >>>> "PCI_NUM_REGIONS - 1". Add a check, just make Coverity happy. >>>> >>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com> >>>> --- >>>> hw/xen/xen_pt_config_init.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c >>>> index 710fe50..3c8b0f1 100644 >>>> --- a/hw/xen/xen_pt_config_init.c >>>> +++ b/hw/xen/xen_pt_config_init.c >>>> @@ -443,6 +443,11 @@ static int xen_pt_bar_reg_read(XenPCIPassthroughState *s, XenPTReg *cfg_entry, >>>> return -1; >>>> } >>>> >>>> + if (index == PCI_ROM_SLOT) { >>>> + XEN_PT_ERR(&s->dev, "Internal error: Access violation at ROM BAR.\n"); >>>> + return -1; >>>> + } >>> >>> Could you please fix the boundaries of the check just above? >>> Also please avoid using PCI_ROM_SLOT for the array index check, simply >>> use PCI_NUM_REGIONS. >>> >> You meaning is changing the below check: >> >> if (index < 0 || index >= PCI_NUM_REGIONS - 1) { >> XEN_PT_ERR(&s->dev, "Internal error: Invalid BAR index [%d].\n", index); >> return -1; >> } >> >> Isn't it? > > that's right > OK, will do, thanks. Regards, -Gonglei
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c index 710fe50..3c8b0f1 100644 --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -443,6 +443,11 @@ static int xen_pt_bar_reg_read(XenPCIPassthroughState *s, XenPTReg *cfg_entry, return -1; } + if (index == PCI_ROM_SLOT) { + XEN_PT_ERR(&s->dev, "Internal error: Access violation at ROM BAR.\n"); + return -1; + } + /* use fixed-up value from kernel sysfs */ *value = base_address_with_flags(&s->real_device.io_regions[index]);