From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50551) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fn1kW-0004Df-GB for qemu-devel@nongnu.org; Tue, 07 Aug 2018 09:10:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fn1kS-0000wc-L8 for qemu-devel@nongnu.org; Tue, 07 Aug 2018 09:10:08 -0400 Date: Tue, 7 Aug 2018 14:09:58 +0100 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Message-ID: <20180807130958.GQ7335@redhat.com> Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= References: <20180807114501.12370-1-peter.maydell@linaro.org> <20180807125223.GF2556@work-vm> <20180807125819.GP7335@redhat.com> <00ac9577-52cc-bf48-f0b8-7d15abb2c21f@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <00ac9577-52cc-bf48-f0b8-7d15abb2c21f@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-3.0] slirp: Correct size check in m_inc() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: "Dr. David Alan Gilbert" , Peter Maydell , Prasad J Pandit , patches@linaro.org, Jan Kiszka , jasowang@redhat.com, qemu-devel@nongnu.org, liqsub1 , Samuel Thibault , qemu-stable@nongnu.org On Tue, Aug 07, 2018 at 03:07:07PM +0200, Thomas Huth wrote: > On 08/07/2018 02:58 PM, Daniel P. Berrang=C3=A9 wrote: > > On Tue, Aug 07, 2018 at 01:52:24PM +0100, Dr. David Alan Gilbert wrot= e: > >> * Peter Maydell (peter.maydell@linaro.org) wrote: > >>> The data in an mbuf buffer is not necessarily at the start of the > >>> allocated buffer. (For instance m_adj() allows data to be trimmed > >>> from the start by just advancing the pointer and reducing the lengt= h.) > >>> This means that the allocated buffer size (m->m_size) and the > >>> amount of space from the m_data pointer to the end of the > >>> buffer (M_ROOM(m)) are not necessarily the same. > >>> > >>> Commit 864036e251f54c9 tried to change the m_inc() function from > >>> taking the new allocated-buffer-size to taking the new room-size, > >>> but forgot to change the initial "do we already have enough space" > >>> check. This meant that if we were trying to extend a buffer which > >>> had a leading gap between the buffer start and the data, we might > >>> incorrectly decide it didn't need to be extended, and then > >>> overrun the end of the buffer, causing memory corruption and > >>> an eventual crash. > >>> > >>> Change the "already big enough?" condition from checking the > >>> argument against m->m_size to checking against M_ROOM(). > >>> This only makes a difference for the callsite in m_cat(); > >>> the other three callsites all start with a freshly allocated > >>> mbuf from m_get(), which will have m->m_size =3D=3D M_ROOM(m). > >>> > >>> Fixes: 864036e251f54c9 > >=20 > > IIUC, this changeset was a security fix for CVE-2018-11806. > >=20 > > Given that the fix was flawed and allowed guest to crash the host > > with a new buffer overrun, it seems we need to get a new CVE allocate= d > > too. >=20 > But 864036e251f54c9 was never part of an official QEMU release, was it? > Or did it go into a stable release already? If not, I think you simply > need both patches to fix the CVE instead. Ah possibly - I didn't look at where 864036e251f54c9 was actually release or not. If its onyl git master, then yeah, we can use the same CVE we already have. Regards, Daniel --=20 |: https://berrange.com -o- https://www.flickr.com/photos/dberran= ge :| |: https://libvirt.org -o- https://fstop138.berrange.c= om :| |: https://entangle-photo.org -o- https://www.instagram.com/dberran= ge :|