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;
