From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50573) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZoNCc-0002gM-FN for qemu-devel@nongnu.org; Mon, 19 Oct 2015 23:03:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZoNCX-00053a-Gw for qemu-devel@nongnu.org; Mon, 19 Oct 2015 23:03:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41192) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZoNCX-00053W-BV for qemu-devel@nongnu.org; Mon, 19 Oct 2015 23:03:01 -0400 References: <5620F082.5040007@redhat.com> From: Jason Wang Message-ID: <5625AEE0.7070908@redhat.com> Date: Tue, 20 Oct 2015 11:02:56 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] eepro100: prevent an infinite loop over same command block List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: P J P , Paolo Bonzini Cc: Qinghao Tang , qemu-devel@nongnu.org On 10/17/2015 01:19 AM, P J P wrote: > +-- On Fri, 16 Oct 2015, Paolo Bonzini wrote --+ > | > + if (s->tx.link == s->cu_offset) > | > + break; > | > | Please update the patch to conform to QEMU's coding standards; braces > | are required even around single-statement blocks. > > Done. Please see an updated patch below. > > === > From bbf7b8914a984b09242e1cafc258bd71cecc47c8 Mon Sep 17 00:00:00 2001 > From: Prasad J Pandit > Date: Fri, 16 Oct 2015 22:43:29 +0530 > Subject: eepro100: prevent an infinite loop over same command block > > action_command() routine executes a chain of commands located > in the Command Block List(CBL). Each Command Block(CB) has a > link to the next CB in the list, given by 's->tx.link'. > This is used in conjunction with the base address 's->cu_base'. > > An infinite loop unfolds if the 'link' to the next CB is > same as the previous one, the loop ends up executing the same > command over and over again. > > Reported-by: Qinghao Tang > Signed-off-by: Prasad J Pandit > --- > hw/net/eepro100.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c > index 60333b7..0e4ad4e 100644 > --- a/hw/net/eepro100.c > +++ b/hw/net/eepro100.c > @@ -863,6 +863,9 @@ static void action_command(EEPRO100State *s) > uint16_t ok_status = STATUS_OK; > s->cb_address = s->cu_base + s->cu_offset; > read_cb(s); > + if (s->tx.link == s->cu_offset) { > + break; > + } > bit_el = ((s->tx.command & COMMAND_EL) != 0); > bit_s = ((s->tx.command & COMMAND_S) != 0); > bit_i = ((s->tx.command & COMMAND_I) != 0); Can this survive if we had a chain like? A->B->A If not, looks like we need to limit the maximum number of commands in a chain? (e.g 256)