From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44657) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fn1TW-0002og-0s for qemu-devel@nongnu.org; Tue, 07 Aug 2018 08:52:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fn1TR-0001m8-Op for qemu-devel@nongnu.org; Tue, 07 Aug 2018 08:52:34 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:51504 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fn1TR-0001ln-JX for qemu-devel@nongnu.org; Tue, 07 Aug 2018 08:52:29 -0400 Date: Tue, 7 Aug 2018 13:52:24 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20180807125223.GF2556@work-vm> References: <20180807114501.12370-1-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180807114501.12370-1-peter.maydell@linaro.org> 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: Peter Maydell Cc: qemu-devel@nongnu.org, patches@linaro.org, jasowang@redhat.com, Samuel Thibault , Jan Kiszka , Prasad J Pandit , liqsub1 * 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 length.) > 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 == M_ROOM(m). > > Fixes: 864036e251f54c9 > Fixes: https://bugs.launchpad.net/qemu/+bug/1785670 > Signed-off-by: Peter Maydell Tested-by: Dr. David Alan Gilbert > --- > slirp/mbuf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/slirp/mbuf.c b/slirp/mbuf.c > index 0c189e1a7bf..1b7868355a3 100644 > --- a/slirp/mbuf.c > +++ b/slirp/mbuf.c > @@ -154,7 +154,7 @@ m_inc(struct mbuf *m, int size) > int datasize; > > /* some compilers throw up on gotos. This one we can fake. */ > - if (m->m_size > size) { > + if (M_ROOM(m) > size) { > return; > } > > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK