* [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects
@ 2015-01-31 7:27 arei.gonglei
2015-01-31 7:27 ` [Qemu-devel] [PATCH 1/2] xen-pt: fix Negative array index read arei.gonglei
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: arei.gonglei @ 2015-01-31 7:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Gonglei, peter.huangpeng, stefano.stabellini
From: Gonglei <arei.gonglei@huawei.com>
Gonglei (2):
xen-pt: fix Negative array index read
xen-pt: fix Out-of-bounds read
hw/xen/xen_pt_config_init.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
--
1.7.12.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/2] xen-pt: fix Negative array index read
2015-01-31 7:27 [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects arei.gonglei
@ 2015-01-31 7:27 ` arei.gonglei
2015-02-10 6:40 ` Stefano Stabellini
2015-01-31 7:27 ` [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read arei.gonglei
2015-02-09 2:37 ` [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects Gonglei
2 siblings, 1 reply; 10+ messages in thread
From: arei.gonglei @ 2015-01-31 7:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Gonglei, peter.huangpeng, stefano.stabellini
From: Gonglei <arei.gonglei@huawei.com>
Coverity spot:
Function xen_pt_bar_offset_to_index() may returns a negative
number (-1) value index, which as an index to array d->io_regions.
Let's directly and simply pass index as an argument to
xen_pt_bar_reg_parse().
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
hw/xen/xen_pt_config_init.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index de9a20f..710fe50 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -360,15 +360,13 @@ static uint64_t xen_pt_get_bar_size(PCIIORegion *r)
}
static XenPTBarFlag xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
- XenPTRegInfo *reg)
+ int index)
{
PCIDevice *d = &s->dev;
XenPTRegion *region = NULL;
PCIIORegion *r;
- int index = 0;
/* check 64bit BAR */
- index = xen_pt_bar_offset_to_index(reg->offset);
if ((0 < index) && (index < PCI_ROM_SLOT)) {
int type = s->real_device.io_regions[index - 1].type;
@@ -422,7 +420,7 @@ static int xen_pt_bar_reg_init(XenPCIPassthroughState *s, XenPTRegInfo *reg,
}
/* set BAR flag */
- s->bases[index].bar_flag = xen_pt_bar_reg_parse(s, reg);
+ s->bases[index].bar_flag = xen_pt_bar_reg_parse(s, index);
if (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED) {
reg_field = XEN_PT_INVALID_REG;
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read
2015-01-31 7:27 [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects arei.gonglei
2015-01-31 7:27 ` [Qemu-devel] [PATCH 1/2] xen-pt: fix Negative array index read arei.gonglei
@ 2015-01-31 7:27 ` arei.gonglei
2015-02-10 6:39 ` Stefano Stabellini
2015-02-09 2:37 ` [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects Gonglei
2 siblings, 1 reply; 10+ messages in thread
From: arei.gonglei @ 2015-01-31 7:27 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Gonglei, peter.huangpeng, stefano.stabellini
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;
+ }
+
/* use fixed-up value from kernel sysfs */
*value = base_address_with_flags(&s->real_device.io_regions[index]);
--
1.7.12.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects
2015-01-31 7:27 [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects arei.gonglei
2015-01-31 7:27 ` [Qemu-devel] [PATCH 1/2] xen-pt: fix Negative array index read arei.gonglei
2015-01-31 7:27 ` [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read arei.gonglei
@ 2015-02-09 2:37 ` Gonglei
2015-02-10 18:52 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2 siblings, 1 reply; 10+ messages in thread
From: Gonglei @ 2015-02-09 2:37 UTC (permalink / raw)
To: Gonglei (Arei)
Cc: qemu-trivial, stefano.stabellini, qemu-devel, Huangpeng (Peter)
On 2015/1/31 15:27, Gonglei (Arei) wrote:
> From: Gonglei <arei.gonglei@huawei.com>
>
>
> Gonglei (2):
> xen-pt: fix Negative array index read
> xen-pt: fix Out-of-bounds read
>
> hw/xen/xen_pt_config_init.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
Ping...
Regards,
-Gonglei
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read
2015-01-31 7:27 ` [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read arei.gonglei
@ 2015-02-10 6:39 ` Stefano Stabellini
2015-02-10 6:49 ` Gonglei
0 siblings, 1 reply; 10+ messages in thread
From: Stefano Stabellini @ 2015-02-10 6:39 UTC (permalink / raw)
To: Gonglei; +Cc: qemu-trivial, stefano.stabellini, qemu-devel, peter.huangpeng
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
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] xen-pt: fix Negative array index read
2015-01-31 7:27 ` [Qemu-devel] [PATCH 1/2] xen-pt: fix Negative array index read arei.gonglei
@ 2015-02-10 6:40 ` Stefano Stabellini
0 siblings, 0 replies; 10+ messages in thread
From: Stefano Stabellini @ 2015-02-10 6:40 UTC (permalink / raw)
To: Gonglei; +Cc: qemu-trivial, stefano.stabellini, qemu-devel, peter.huangpeng
On Sat, 31 Jan 2015, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
>
> Coverity spot:
> Function xen_pt_bar_offset_to_index() may returns a negative
> number (-1) value index, which as an index to array d->io_regions.
>
> Let's directly and simply pass index as an argument to
> xen_pt_bar_reg_parse().
>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> hw/xen/xen_pt_config_init.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index de9a20f..710fe50 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -360,15 +360,13 @@ static uint64_t xen_pt_get_bar_size(PCIIORegion *r)
> }
>
> static XenPTBarFlag xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
> - XenPTRegInfo *reg)
> + int index)
> {
> PCIDevice *d = &s->dev;
> XenPTRegion *region = NULL;
> PCIIORegion *r;
> - int index = 0;
>
> /* check 64bit BAR */
> - index = xen_pt_bar_offset_to_index(reg->offset);
> if ((0 < index) && (index < PCI_ROM_SLOT)) {
> int type = s->real_device.io_regions[index - 1].type;
>
> @@ -422,7 +420,7 @@ static int xen_pt_bar_reg_init(XenPCIPassthroughState *s, XenPTRegInfo *reg,
> }
>
> /* set BAR flag */
> - s->bases[index].bar_flag = xen_pt_bar_reg_parse(s, reg);
> + s->bases[index].bar_flag = xen_pt_bar_reg_parse(s, index);
> if (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED) {
> reg_field = XEN_PT_INVALID_REG;
> }
> --
> 1.7.12.4
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read
2015-02-10 6:39 ` Stefano Stabellini
@ 2015-02-10 6:49 ` Gonglei
2015-02-10 7:00 ` Stefano Stabellini
0 siblings, 1 reply; 10+ messages in thread
From: Gonglei @ 2015-02-10 6:49 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: qemu-trivial, qemu-devel, peter.huangpeng
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read
2015-02-10 6:49 ` Gonglei
@ 2015-02-10 7:00 ` Stefano Stabellini
2015-02-10 7:19 ` Gonglei
0 siblings, 1 reply; 10+ messages in thread
From: Stefano Stabellini @ 2015-02-10 7:00 UTC (permalink / raw)
To: Gonglei; +Cc: qemu-trivial, peter.huangpeng, qemu-devel, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read
2015-02-10 7:00 ` Stefano Stabellini
@ 2015-02-10 7:19 ` Gonglei
0 siblings, 0 replies; 10+ messages in thread
From: Gonglei @ 2015-02-10 7:19 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: qemu-trivial, qemu-devel, peter.huangpeng
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH 0/2] xen_pt: fix two Coverity defects
2015-02-09 2:37 ` [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects Gonglei
@ 2015-02-10 18:52 ` Michael Tokarev
0 siblings, 0 replies; 10+ messages in thread
From: Michael Tokarev @ 2015-02-10 18:52 UTC (permalink / raw)
To: Gonglei; +Cc: qemu-trivial, Huangpeng (Peter), qemu-devel, stefano.stabellini
09.02.2015 05:37, Gonglei wrote:
> On 2015/1/31 15:27, Gonglei (Arei) wrote:
>
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> Gonglei (2):
>> xen-pt: fix Negative array index read
>> xen-pt: fix Out-of-bounds read
>>
>> hw/xen/xen_pt_config_init.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> Ping...
I thought you want to change the second patch, did I misunderstand?
Thanks,
/mjt
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-02-10 18:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-31 7:27 [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects arei.gonglei
2015-01-31 7:27 ` [Qemu-devel] [PATCH 1/2] xen-pt: fix Negative array index read arei.gonglei
2015-02-10 6:40 ` Stefano Stabellini
2015-01-31 7:27 ` [Qemu-devel] [PATCH 2/2] xen-pt: fix Out-of-bounds read arei.gonglei
2015-02-10 6:39 ` Stefano Stabellini
2015-02-10 6:49 ` Gonglei
2015-02-10 7:00 ` Stefano Stabellini
2015-02-10 7:19 ` Gonglei
2015-02-09 2:37 ` [Qemu-devel] [PATCH 0/2] xen_pt: fix two Coverity defects Gonglei
2015-02-10 18:52 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.