I think the important issue is that we should modify ne2000_receive > @@ -247,6 +247,7ssize_t ne2000_receive(NetClientState *nc, const uint8_t > *buf, size_t size_) > - if (index <= s->stop) > + if (index < s->stop)
-----邮件原件----- 发件人: Jason Wang [mailto:jasow...@redhat.com] 发送时间: 2016年2月5日 17:05 收件人: P J P; QEMU Developers 抄送: yanghongke; Prasad J Pandit 主题: Re: [PATCH] net: ne2000: check ring buffer control registers On 02/02/2016 10:29 PM, P J P wrote: > From: Prasad J Pandit <p...@fedoraproject.org> > > Ne2000 NIC uses ring buffer of NE2000_MEM_SIZE(49152) bytes to process > network packets. Four registers PSTART, PSTOP, CURPAGE and BOUNDARY > are used to control ring buffer access. Setting these registers to > invalid values could lead to infinite loop or OOB r/w access issues. > Add checks to avoid it. > > Reported-by: Yang Hongke <yanghon...@huawei.com> > Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> > --- > hw/net/ne2000.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c index 9dd0c67..b032212 > 100644 > --- a/hw/net/ne2000.c > +++ b/hw/net/ne2000.c > @@ -269,6 +269,7 @@ ssize_t ne2000_receive(NetClientState *nc, const > uint8_t *buf, size_t size_) > > static void ne2000_ioport_write(void *opaque, uint32_t addr, uint32_t > val) { > + uint32_t v; > NE2000State *s = opaque; > int offset, page, index; > > @@ -309,17 +310,20 @@ static void ne2000_ioport_write(void *opaque, uint32_t > addr, uint32_t val) > offset = addr | (page << 4); > switch(offset) { > case EN0_STARTPG: > - if (val << 8 <= NE2000_PMEM_END) { > - s->start = val << 8; > + v = val << 8; > + if (v < NE2000_PMEM_END && v < s->stop) { I suspect this could even work. Consider after realizing, s->stop is zero, any attempt to set STARTPG will fail? > + s->start = v; > } > break; > case EN0_STOPPG: > - if (val << 8 <= NE2000_PMEM_END) { > - s->stop = val << 8; > + v = val << 8; > + if (v <= NE2000_PMEM_END && v > s->start) { > + s->stop = v; > } > break; > case EN0_BOUNDARY: > - if (val << 8 < NE2000_PMEM_END) { > + v = val << 8; > + if (v >= s->start && v <= s->stop) { > s->boundary = val; This may not be sufficient, consider: set start to 1 set stop to 100 set boundary to 50 then set stop to 10 I'm thinking maybe we need check during receiving like what we did in dd793a74882477ca38d49e191110c17dfee51dcc? > } > break; > @@ -362,7 +366,8 @@ static void ne2000_ioport_write(void *opaque, uint32_t > addr, uint32_t val) > s->phys[offset - EN1_PHYS] = val; > break; > case EN1_CURPAG: > - if (val << 8 < NE2000_PMEM_END) { > + v = val << 8; > + if (v >= s->start && v <= s->stop) { > s->curpag = val; > } > break;
