Bug#400874: I hope this is the right fix...
On Mon, Dec 04, 2006 at 10:46:18AM +0100, Michael Vogt wrote: > Unfortunately this patch is not enough because BigBuf is deleted [...] > right now. But testing feedback would be great :) I've verified that the problem still persists with the apt which is currently in the archive. I've also tested apt with you patch (debsrcrec.diff) and verified that it solves the problem. Thanks for a proper patch! Hope to see this uploaded soon. :) -- Regards, Andreas Henriksson -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Bug#400874: I hope this is the right fix...
On Thu, Nov 30, 2006 at 10:56:52AM +0100, Andreas Henriksson wrote: > On Thu, Nov 30, 2006 at 08:49:26AM +0100, Jens Seidel wrote: > > If the buffer needs to be longer by one than Bins you probably also need > > +if (Bins.length() >= sizeof(Buffer)) > > Good catch, thanks! > > Updated patch attached. Thanks for your patch. Unfortunately this patch is not enough because BigBuf is deleted in the function but TokSplitString() does not make a copy of the buffer it is passed but just modifies it. So we return already deleted memory. I send a proposed patch that hopefully fixes this issue. I'm currently at the lsb-meeting so I'm a bit limited in my resources right now. But testing feedback would be great :) Cheers, Michael -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Bug#400874: I hope this is the right fix...
> Updated patch attached. > > > I wonder what the second part of this is good for: > > if (Bins.empty() == true || Bins.length() >= 102400) > return 0; > > Oh, well that's a high enough number that it probably won't be(come) > a real world problem and if we're getting that nitpicky at fixing > surrounding issues we should probably start by checking if the BigBuf > memory allocation failed first. :) Aptsource-fix2.diff works for me as well (here amd64). Hope it helps, -- bye, - Nacho http://criptonita.com/~nacho -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Bug#400874: I hope this is the right fix...
* Andreas Henriksson ([EMAIL PROTECTED]) [061130 02:04]: > Updated patch attached. > > > I wonder what the second part of this is good for: > > if (Bins.empty() == true || Bins.length() >= 102400) > return 0; > > Oh, well that's a high enough number that it probably won't be(come) > a real world problem and if we're getting that nitpicky at fixing > surrounding issues we should probably start by checking if the BigBuf > memory allocation failed first. :) Thank you very much for this patch. I can confirm that this patch works for me (amd64). In case it is helpful, I'm offering to NMU apt (well, unless someone stops me or is faster, I'll do that as soon as apt appears on my regular list of >= 10 days old RC bugs). Cheers, Andi -- http://home.arcor.de/andreas-barth/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Bug#400874: I hope this is the right fix...
On Thu, Nov 30, 2006 at 08:49:26AM +0100, Jens Seidel wrote:
> If the buffer needs to be longer by one than Bins you probably also need
> +if (Bins.length() >= sizeof(Buffer))
Good catch, thanks!
Updated patch attached.
I wonder what the second part of this is good for:
if (Bins.empty() == true || Bins.length() >= 102400)
return 0;
Oh, well that's a high enough number that it probably won't be(come)
a real world problem and if we're getting that nitpicky at fixing
surrounding issues we should probably start by checking if the BigBuf
memory allocation failed first. :)
--
Regards,
Andreas Henriksson
diff -uri apt-0.6.46.3/apt-pkg/deb/debsrcrecords.cc
apt-0.6.46.3-fixed/apt-pkg/deb/debsrcrecords.cc
--- apt-0.6.46.3/apt-pkg/deb/debsrcrecords.cc 2006-03-02 14:44:28.0
+0100
+++ apt-0.6.46.3-fixed/apt-pkg/deb/debsrcrecords.cc 2006-11-30
10:35:18.0 +0100
@@ -38,9 +38,9 @@
// is large, to avoid a performance penalty
char *BigBuf = NULL;
char *Buf;
- if (Bins.length() > sizeof(Buffer))
+ if (Bins.length() >= sizeof(Buffer))
{
- BigBuf = new char[Bins.length()];
+ BigBuf = new char[Bins.length()+1];
Buf = BigBuf;
}
else
Bug#400874: I hope this is the right fix...
On Thu, Nov 30, 2006 at 01:06:16AM +0100, Andreas Henriksson wrote:
> Patch attached.
>
>
> Regards,
> Andreas Henriksson
> diff -ur apt-0.6.46.3/apt-pkg/deb/debsrcrecords.cc
> apt-0.6.46.3.fix/apt-pkg/deb/debsrcrecords.cc
> --- apt-0.6.46.3/apt-pkg/deb/debsrcrecords.cc 2006-03-02 14:44:28.0
> +0100
> +++ apt-0.6.46.3.fix/apt-pkg/deb/debsrcrecords.cc 2006-11-30
> 00:38:19.0 +0100
> @@ -40,7 +40,7 @@
> char *Buf;
> if (Bins.length() > sizeof(Buffer))
> {
> - BigBuf = new char[Bins.length()];
> + BigBuf = new char[Bins.length()+1];
>Buf = BigBuf;
> }
> else
If the buffer needs to be longer by one than Bins you probably also need
-if (Bins.length() > sizeof(Buffer))
+if (Bins.length() >= sizeof(Buffer))
Jens
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
Bug#400874: I hope this is the right fix...
tags 400874 + patch
thanks
Here's the off-by-one fix for apt. Now that it's solved it seems so
obvious... valgrind was right! (Could someone please rerun valgrind with
this patch applied?!)
Please verify for correctness, this patch "works for me" (and in my
current state seems to be the right fix, I'll try to have a closer look
at it tomorrow or maybe later...)
Problem described in:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=400874;msg=36
Workaround:
http://fatal.se/tmp/aptsource-workaround.diff
Probably/hopefully this is the "real" fix:
http://fatal.se/tmp/aptsource-fix.diff
(this one also attached)
Patch attached.
Regards,
Andreas Henriksson
diff -ur apt-0.6.46.3/apt-pkg/deb/debsrcrecords.cc apt-0.6.46.3.fix/apt-pkg/deb/debsrcrecords.cc
--- apt-0.6.46.3/apt-pkg/deb/debsrcrecords.cc 2006-03-02 14:44:28.0 +0100
+++ apt-0.6.46.3.fix/apt-pkg/deb/debsrcrecords.cc 2006-11-30 00:38:19.0 +0100
@@ -40,7 +40,7 @@
char *Buf;
if (Bins.length() > sizeof(Buffer))
{
- BigBuf = new char[Bins.length()];
+ BigBuf = new char[Bins.length()+1];
Buf = BigBuf;
}
else
