From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44171) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atYBb-0003A3-8r for qemu-devel@nongnu.org; Fri, 22 Apr 2016 06:19:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1atYBa-000538-8F for qemu-devel@nongnu.org; Fri, 22 Apr 2016 06:19:43 -0400 Date: Fri, 22 Apr 2016 12:19:33 +0200 From: Kevin Wolf Message-ID: <20160422101933.GB4237@noname.redhat.com> References: <1461249750-31928-1-git-send-email-eblake@redhat.com> <20160422070325.GA4237@noname.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH for-2.6?] nbd: Don't mishandle unaligned client requests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Eric Blake , QEMU Developers , Paolo Bonzini , Qemu-block , qemu-stable Am 22.04.2016 um 11:29 hat Peter Maydell geschrieben: > On 22 April 2016 at 08:03, Kevin Wolf wrote: > > Am 21.04.2016 um 18:28 hat Peter Maydell geschrieben: > >> On 21 April 2016 at 15:42, Eric Blake wrote: > >> > The NBD protocol does not (yet) force any alignment constraints > >> > on clients. Even though qemu NBD clients always send requests > >> > that are aligned to 512 bytes, we must be prepared for non-qemu > >> > clients that don't care about alignment (even if it means they > >> > are less efficient). Our use of blk_read() and blk_write() was > >> > silently operating on the wrong file offsets when the client > >> > made an unaligned request, corrupting the client's data (but > >> > as the client already has control over the file we are serving, > >> > I don't think it is a security hole, per se, just a data > >> > corruption bug). > >> > > >> > Note that in the case of NBD_CMD_READ, an unaligned length could > >> > cause us to return up to 511 bytes of uninitialized trailing > >> > garbage from blk_try_blockalign() - hopefully nothing sensitive > >> > from the heap's prior usage is ever leaked in that manner. > >> > > >> > Signed-off-by: Eric Blake > >> > --- > >> > > >> > It's late for 2.6, but as a data corruption bug fix, I think > >> > it's worth having if there is still time. > >> > >> I want to tag rc3 today, but since it looks like there's going to > >> be an rc4 for the virtio handler bug this can probably go into rc4 > >> if it gets review. > > > > Reviewed-by: Kevin Wolf > > > > Peter, do you want a pull request (which I would have to do because > > Paolo is away) or are you going to apply the patch directly? > > If you're happy on the review and testing front I can apply it > to master directly (I won't be able to do any testing beyond > running "make check".) I am. It's a trivial patch anyway, but I've also tested it with qemu-iotests and by installing a guest on an NBD device. So if you like, you can also add: Tested-by: Kevin Wolf