| Message ID | 1482459390-14386-1-git-send-email-caoj.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Dec 23, 2016 at 10:16:30AM +0800, Cao jin wrote: > "size >= 8" connote "size > 0" > > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> Isn't the point to check for overflows? > --- > hw/pci/pcie.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 39b10b852d91..f864c5cd5458 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -668,7 +668,6 @@ void pcie_add_capability(PCIDevice *dev, > uint16_t next; > > assert(offset >= PCI_CONFIG_SPACE_SIZE); > - assert(offset < offset + size); > assert(offset + size <= PCIE_CONFIG_SPACE_SIZE); > assert(size >= 8); > assert(pci_is_express(dev)); > -- > 2.1.0 > >
On 01/10/2017 06:37 AM, Michael S. Tsirkin wrote: > On Fri, Dec 23, 2016 at 10:16:30AM +0800, Cao jin wrote: >> "size >= 8" connote "size > 0" >> >> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > > Isn't the point to check for overflows? > Make sense. If it is intended to check overflows, the following sequence would make more sense: assert(offset >= PCI_CONFIG_SPACE_SIZE); assert(size >= 8); assert(offset < offset + size); assert(offset + size <= PCIE_CONFIG_SPACE_SIZE); or else, size 0 will pass the assert(offset < offset + size) first and hit assert(size >= 8)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 39b10b852d91..f864c5cd5458 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -668,7 +668,6 @@ void pcie_add_capability(PCIDevice *dev, uint16_t next; assert(offset >= PCI_CONFIG_SPACE_SIZE); - assert(offset < offset + size); assert(offset + size <= PCIE_CONFIG_SPACE_SIZE); assert(size >= 8); assert(pci_is_express(dev));
"size >= 8" connote "size > 0" Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> --- hw/pci/pcie.c | 1 - 1 file changed, 1 deletion(-)