From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36901) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WV23b-0003UK-Pa for qemu-devel@nongnu.org; Tue, 01 Apr 2014 13:01:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WV23W-0004IH-VZ for qemu-devel@nongnu.org; Tue, 01 Apr 2014 13:01:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37649) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WV23W-0004I5-Lt for qemu-devel@nongnu.org; Tue, 01 Apr 2014 13:00:58 -0400 Date: Tue, 1 Apr 2014 18:00:51 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20140401170051.GQ2411@work-vm> References: <1396371187-8567-1-git-send-email-peter.maydell@linaro.org> <1396371187-8567-2-git-send-email-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1396371187-8567-2-git-send-email-peter.maydell@linaro.org> Subject: Re: [Qemu-devel] [PATCH 1/5] hw/net/stellaris_enet: Restructure tx_fifo code to avoid buffer overrun List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: "Michael S. Tsirkin" , qemu-devel@nongnu.org, patches@linaro.org * Peter Maydell (peter.maydell@linaro.org) wrote: > The current tx_fifo code has a corner case where the guest can overrun > the fifo buffer: if automatic CRCs are disabled we allow the guest to write > the CRC word even if there isn't actually space for it in the FIFO. > The datasheet is unclear about exactly how the hardware deals with this > situation; the most plausible answer seems to be that the CRC word is > just lost. > > Implement this fix by separating the "can we stuff another word in the > FIFO" logic from the "should we transmit the packet now" check. This > also moves us closer to the real hardware, which has a number of ways > it can be configured to trigger sending the packet, some of which we > don't implement. > > Signed-off-by: Peter Maydell Reviewed-by: Dr. David Alan Gilbert > --- > hw/net/stellaris_enet.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c > index d04e6a4..bd844cd 100644 > --- a/hw/net/stellaris_enet.c > +++ b/hw/net/stellaris_enet.c > @@ -253,10 +253,12 @@ static void stellaris_enet_write(void *opaque, hwaddr offset, > s->tx_fifo[s->tx_fifo_len++] = value >> 24; > } > } else { > - s->tx_fifo[s->tx_fifo_len++] = value; > - s->tx_fifo[s->tx_fifo_len++] = value >> 8; > - s->tx_fifo[s->tx_fifo_len++] = value >> 16; > - s->tx_fifo[s->tx_fifo_len++] = value >> 24; > + if (s->tx_fifo_len + 4 <= ARRAY_SIZE(s->tx_fifo)) { > + s->tx_fifo[s->tx_fifo_len++] = value; > + s->tx_fifo[s->tx_fifo_len++] = value >> 8; > + s->tx_fifo[s->tx_fifo_len++] = value >> 16; > + s->tx_fifo[s->tx_fifo_len++] = value >> 24; > + } > if (s->tx_fifo_len >= s->tx_frame_len) { > /* We don't implement explicit CRC, so just chop it off. */ > if ((s->tctl & SE_TCTL_CRC) == 0) > -- > 1.9.0 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK