* [Qemu-devel] [PATCH] net: ne2000: fix bounds check in ioport operations @ 2015-12-30 7:44 P J P 2015-12-30 7:44 ` P J P 0 siblings, 1 reply; 3+ messages in thread From: P J P @ 2015-12-30 7:44 UTC (permalink / raw) To: QEMU Developers; +Cc: Jason Wang, Prasad J Pandit, Ling Liu From: Prasad J Pandit <pjp@fedoraproject.org> Hello, An OOB r/w issue in ne2000 device emulation was reported by Mr Ling Liu, CC'd here. The issue occurs while doing ne2000 ioport r/w operations, due to incorrect array bounds checks. Below is a proposed (tested)patch to fix this issue. Does it look okay? Thank you. -- Prasad J Pandit (1): net: ne2000: fix bounds check in ioport operations hw/net/ne2000.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) -- 2.4.3 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH] net: ne2000: fix bounds check in ioport operations 2015-12-30 7:44 [Qemu-devel] [PATCH] net: ne2000: fix bounds check in ioport operations P J P @ 2015-12-30 7:44 ` P J P 2015-12-31 3:10 ` Jason Wang 0 siblings, 1 reply; 3+ messages in thread From: P J P @ 2015-12-30 7:44 UTC (permalink / raw) To: QEMU Developers; +Cc: Jason Wang, Prasad J Pandit, Ling Liu From: Prasad J Pandit <pjp@fedoraproject.org> While doing ioport r/w oprations, ne2000 device emulation suffers from OOB r/w error. Update respective array bounds check to avoid OOB access. Reported-by: Ling Liu <liuling-it@360.cn> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> --- hw/net/ne2000.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c index 010f9ef..42e4804 100644 --- a/hw/net/ne2000.c +++ b/hw/net/ne2000.c @@ -447,9 +447,10 @@ static uint32_t ne2000_ioport_read(void *opaque, uint32_t addr) static inline void ne2000_mem_writeb(NE2000State *s, uint32_t addr, uint32_t val) { - if (addr < 32 || - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { - s->mem[addr] = val; + if (addr < 32 + || (addr >= NE2000_PMEM_START + && addr <= NE2000_MEM_SIZE - sizeof(uint8_t))) { + s->mem[addr] = (uint8_t)val; } } @@ -457,8 +458,9 @@ static inline void ne2000_mem_writew(NE2000State *s, uint32_t addr, uint32_t val) { addr &= ~1; /* XXX: check exact behaviour if not even */ - if (addr < 32 || - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { + if (addr < 32 + || (addr >= NE2000_PMEM_START + && addr <= NE2000_MEM_SIZE - sizeof(uint16_t))) { *(uint16_t *)(s->mem + addr) = cpu_to_le16(val); } } @@ -467,17 +469,19 @@ static inline void ne2000_mem_writel(NE2000State *s, uint32_t addr, uint32_t val) { addr &= ~1; /* XXX: check exact behaviour if not even */ - if (addr < 32 || - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { + if (addr < 32 + || (addr >= NE2000_PMEM_START + && addr <= NE2000_MEM_SIZE - sizeof(uint32_t))) { stl_le_p(s->mem + addr, val); } } static inline uint32_t ne2000_mem_readb(NE2000State *s, uint32_t addr) { - if (addr < 32 || - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { - return s->mem[addr]; + if (addr < 32 + || (addr >= NE2000_PMEM_START + && addr <= NE2000_MEM_SIZE - sizeof(uint8_t))) { + return (uint8_t)s->mem[addr]; } else { return 0xff; } @@ -486,8 +490,9 @@ static inline uint32_t ne2000_mem_readb(NE2000State *s, uint32_t addr) static inline uint32_t ne2000_mem_readw(NE2000State *s, uint32_t addr) { addr &= ~1; /* XXX: check exact behaviour if not even */ - if (addr < 32 || - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { + if (addr < 32 + || (addr >= NE2000_PMEM_START + && addr <= NE2000_MEM_SIZE - sizeof(uint16_t))) { return le16_to_cpu(*(uint16_t *)(s->mem + addr)); } else { return 0xffff; @@ -497,8 +502,9 @@ static inline uint32_t ne2000_mem_readw(NE2000State *s, uint32_t addr) static inline uint32_t ne2000_mem_readl(NE2000State *s, uint32_t addr) { addr &= ~1; /* XXX: check exact behaviour if not even */ - if (addr < 32 || - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { + if (addr < 32 + || (addr >= NE2000_PMEM_START + && addr <= NE2000_MEM_SIZE - sizeof(uint32_t))) { return ldl_le_p(s->mem + addr); } else { return 0xffffffff; -- 2.4.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] net: ne2000: fix bounds check in ioport operations 2015-12-30 7:44 ` P J P @ 2015-12-31 3:10 ` Jason Wang 0 siblings, 0 replies; 3+ messages in thread From: Jason Wang @ 2015-12-31 3:10 UTC (permalink / raw) To: P J P, QEMU Developers; +Cc: Prasad J Pandit, Ling Liu On 12/30/2015 03:44 PM, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While doing ioport r/w oprations, ne2000 device emulation suffers > from OOB r/w error. Update respective array bounds check to avoid > OOB access. > > Reported-by: Ling Liu <liuling-it@360.cn> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/net/ne2000.c | 34 ++++++++++++++++++++-------------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c > index 010f9ef..42e4804 100644 > --- a/hw/net/ne2000.c > +++ b/hw/net/ne2000.c > @@ -447,9 +447,10 @@ static uint32_t ne2000_ioport_read(void *opaque, uint32_t addr) > static inline void ne2000_mem_writeb(NE2000State *s, uint32_t addr, > uint32_t val) > { > - if (addr < 32 || > - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { > - s->mem[addr] = val; > + if (addr < 32 > + || (addr >= NE2000_PMEM_START > + && addr <= NE2000_MEM_SIZE - sizeof(uint8_t))) { Patch makes sense just the check looks odd. I'd prefer something like: uint8_t data = val; if (... || addr + sizeof(data) <= NE2000_MEM_SIZE) Thanks > + s->mem[addr] = (uint8_t)val; > } > } > > @@ -457,8 +458,9 @@ static inline void ne2000_mem_writew(NE2000State *s, uint32_t addr, > uint32_t val) > { > addr &= ~1; /* XXX: check exact behaviour if not even */ > - if (addr < 32 || > - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { > + if (addr < 32 > + || (addr >= NE2000_PMEM_START > + && addr <= NE2000_MEM_SIZE - sizeof(uint16_t))) { > *(uint16_t *)(s->mem + addr) = cpu_to_le16(val); > } > } > @@ -467,17 +469,19 @@ static inline void ne2000_mem_writel(NE2000State *s, uint32_t addr, > uint32_t val) > { > addr &= ~1; /* XXX: check exact behaviour if not even */ > - if (addr < 32 || > - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { > + if (addr < 32 > + || (addr >= NE2000_PMEM_START > + && addr <= NE2000_MEM_SIZE - sizeof(uint32_t))) { > stl_le_p(s->mem + addr, val); > } > } > > static inline uint32_t ne2000_mem_readb(NE2000State *s, uint32_t addr) > { > - if (addr < 32 || > - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { > - return s->mem[addr]; > + if (addr < 32 > + || (addr >= NE2000_PMEM_START > + && addr <= NE2000_MEM_SIZE - sizeof(uint8_t))) { > + return (uint8_t)s->mem[addr]; > } else { > return 0xff; > } > @@ -486,8 +490,9 @@ static inline uint32_t ne2000_mem_readb(NE2000State *s, uint32_t addr) > static inline uint32_t ne2000_mem_readw(NE2000State *s, uint32_t addr) > { > addr &= ~1; /* XXX: check exact behaviour if not even */ > - if (addr < 32 || > - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { > + if (addr < 32 > + || (addr >= NE2000_PMEM_START > + && addr <= NE2000_MEM_SIZE - sizeof(uint16_t))) { > return le16_to_cpu(*(uint16_t *)(s->mem + addr)); > } else { > return 0xffff; > @@ -497,8 +502,9 @@ static inline uint32_t ne2000_mem_readw(NE2000State *s, uint32_t addr) > static inline uint32_t ne2000_mem_readl(NE2000State *s, uint32_t addr) > { > addr &= ~1; /* XXX: check exact behaviour if not even */ > - if (addr < 32 || > - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { > + if (addr < 32 > + || (addr >= NE2000_PMEM_START > + && addr <= NE2000_MEM_SIZE - sizeof(uint32_t))) { > return ldl_le_p(s->mem + addr); > } else { > return 0xffffffff; ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-12-31 3:11 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-30 7:44 [Qemu-devel] [PATCH] net: ne2000: fix bounds check in ioport operations P J P 2015-12-30 7:44 ` P J P 2015-12-31 3:10 ` Jason Wang
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.