From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chunyan Liu Subject: Re: question about SIGSEGV in datacopier_readable in libxl_aoutil.c [and 1 more messages] Date: Wed, 4 Sep 2013 16:15:11 +0800 Message-ID: References: <1378195018.7651.55.camel@kazak.uk.xensource.com> <5225AA83.5080005@citrix.com> <21029.62118.393604.371370@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0963382734611063395==" Return-path: In-Reply-To: <21029.62118.393604.371370@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Andrew Cooper , "xen-devel@lists.xensource.com" , Ian Campbell List-Id: xen-devel@lists.xenproject.org --===============0963382734611063395== Content-Type: multipart/alternative; boundary=001a11c384627e027d04e58a6773 --001a11c384627e027d04e58a6773 Content-Type: text/plain; charset=UTF-8 2013/9/3 Ian Jackson > Ian Campbell writes ("Re: [Xen-devel] question about SIGSEGV in > datacopier_readable in libxl_aoutil.c"): > > On Tue, 2013-09-03 at 15:01 +0800, Chunyan Liu wrote: > > > if (!buf || buf->used >= sizeof(buf->buf)) { > > > buf = malloc(sizeof(*buf)); > ... > > > ==7510== Syscall param read(buf) points to unaddressable byte(s) > ... > > > ==7510== Address 0x18a409ec is 0 bytes after a block of size 28 > alloc'd > > > ==7510== at 0x4C26FFB: calloc (in > /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > > > ==7510== by 0x14AAECB6: libxl__zalloc (libxl_internal.c:83) > > > ==7510== by 0x14AB33B0: libxl__datacopier_prefixdata > (libxl_aoutils.c:92) > > I think this is my fault. Please try this patch. > > Thanks, > Ian. > > commit 25cd65c97b733d5892b62c3ffae0887f426398ec > Author: Ian Jackson > Date: Tue Sep 3 13:41:46 2013 +0100 > > libxl: Do not generate short block in libxl__datacopier_prefixdata > > libxl__datacopier_prefixdata would prepend a deliberately short block > (not just a half-full one, but one with a short buffer) to the > dc->bufs queue. However, this is wrong because datacopier_readable > will find it and try to continue to fill it up. > > Instead, allocate a full-sized buffer. > > Signed-off-by: Ian Jackson > > diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c > index 983a60a..b4eb6e5 100644 > --- a/tools/libxl/libxl_aoutils.c > +++ b/tools/libxl/libxl_aoutils.c > @@ -89,7 +89,7 @@ void libxl__datacopier_prefixdata(libxl__egc *egc, > libxl__datacopier_state *dc, > > assert(len < dc->maxsz - dc->used); > > - buf = libxl__zalloc(NOGC, sizeof(*buf) - sizeof(buf->buf) + len); > + buf = libxl__zalloc(NOGC, sizeof(*buf)); > buf->used = len; > memcpy(buf->buf, data, len); > > Tried the patch. It worked. Thanks. > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > > --001a11c384627e027d04e58a6773 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable



2013/9/3 Ian Jackson <Ian.Jackson@eu.citrix.com>
Ian Campbell writes ("Re: [Xen-devel] question about SIGSEGV in dataco= pier_readable in libxl_aoutil.c"):
> On Tue, 2013-09-03 at 15:01 +0800, Chunyan Liu wrote:
> > =C2=A0 =C2=A0 =C2=A0 if (!buf || buf->used >=3D sizeof(buf-= >buf)) {
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 buf =3D malloc(sizeof(*= buf));
...
> > =3D=3D7510=3D=3D Syscall param read(buf) points to unaddressable = byte(s)
...
> > =3D=3D7510=3D=3D =C2=A0Address 0x18a409ec is 0 bytes after a bloc= k of size 28 alloc'd
> > =3D=3D7510=3D=3D =C2=A0 =C2=A0at 0x4C26FFB: calloc (in /usr/lib64= /valgrind/vgpreload_memcheck-amd64-linux.so)
> > =3D=3D7510=3D=3D =C2=A0 =C2=A0by 0x14AAECB6: libxl__zalloc (libxl= _internal.c:83)
> > =3D=3D7510=3D=3D =C2=A0 =C2=A0by 0x14AB33B0: libxl__datacopier_pr= efixdata (libxl_aoutils.c:92)

I think this is my fault. =C2=A0Please try this patch.

Thanks,
Ian.

commit 25cd65c97b733d5892b62c3ffae0887f426398ec
Author: Ian Jackson <ian.ja= ckson@eu.citrix.com>
Date: =C2=A0 Tue Sep 3 13:41:46 2013 +0100

=C2=A0 =C2=A0 libxl: Do not generate short block in libxl__datacopier_prefi= xdata

=C2=A0 =C2=A0 libxl__datacopier_prefixdata would prepend a deliberately sho= rt block
=C2=A0 =C2=A0 (not just a half-full one, but one with a short buffer) to th= e
=C2=A0 =C2=A0 dc->bufs queue. =C2=A0However, this is wrong because datac= opier_readable
=C2=A0 =C2=A0 will find it and try to continue to fill it up.

=C2=A0 =C2=A0 Instead, allocate a full-sized buffer.

=C2=A0 =C2=A0 Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 983a60a..b4eb6e5 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -89,7 +89,7 @@ void libxl__datacopier_prefixdata(libxl__egc *egc, libxl_= _datacopier_state *dc,

=C2=A0 =C2=A0 =C2=A0assert(len < dc->maxsz - dc->used);

- =C2=A0 =C2=A0buf =3D libxl__zalloc(NOGC, sizeof(*buf) - sizeof(buf->bu= f) + len);
+ =C2=A0 =C2=A0buf =3D libxl__zalloc(NOGC, sizeof(*buf));
=C2=A0 =C2=A0 =C2=A0buf->used =3D len;
=C2=A0 =C2=A0 =C2=A0memcpy(buf->buf, data, len);


Tried the patch. It worked. Thanks.
=C2=A0
=
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.x= en.org/xen-devel


--001a11c384627e027d04e58a6773-- --===============0963382734611063395== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============0963382734611063395==--