From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55685) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aY3dq-0005Ii-0e for qemu-devel@nongnu.org; Mon, 22 Feb 2016 22:28:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aY3dk-0006JD-UA for qemu-devel@nongnu.org; Mon, 22 Feb 2016 22:28:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58830) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aY3dk-0006J0-PE for qemu-devel@nongnu.org; Mon, 22 Feb 2016 22:27:56 -0500 References: <1454423392-7732-1-git-send-email-ppandit@redhat.com> <56B465B7.8000201@redhat.com> From: Jason Wang Message-ID: <56CBD1AF.6090802@redhat.com> Date: Tue, 23 Feb 2016 11:27:43 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net: ne2000: check ring buffer control registers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: P J P Cc: Yang Hongke , QEMU Developers On 02/09/2016 02:47 PM, P J P wrote: > Hello Jason, > > +-- On Fri, 5 Feb 2016, Jason Wang wrote --+ > | I suspect this could even work. Consider after realizing, s->stop is > | zero, any attempt to set STARTPG will fail? > > Ie after 'pci_ne2000_realize'? It does not seem to set or reset s->stop > register. I mean with your patch, driver will only be allowed to set EN0_STOPPG before EN0_STARTPG. So if a driver want to set STARTPG first, the check + if (v < NE2000_PMEM_END && v < s->stop) { will prevent the driver from working correctly since s->stop is zero here. > > | This may not be sufficient, consider: > | > | set start to 1 > | set stop to 100 > | set boundary to 50 > | then set stop to 10 > > I think any attempts to define the ring buffer limits should reset > 'boundary' and 'curpag' registers to s->start(STARTPG). I wonder if a driver > should be allowed to fiddle with the ring buffers location inside contorller's > memory. It does not seem right. Well, I think we could not assume the behavior of a driver, especially consider it may be malicious. > > | I'm thinking maybe we need check during receiving like what we did in > | dd793a74882477ca38d49e191110c17dfee51dcc? > > Check if (s->start == s->stop) at each receive call? Or in ne2000_buffer_full()? > > -- > - P J P > 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F >