Bug#669427: apt segfaults on s390x

[David Kalnischkies <kalnischkies+debian@gmail.com>]

On Tue, May 1, 2012 at 4:53 PM, Philipp Kern <pkern@debian.org> wrote:
> On Fri, Apr 20, 2012 at 11:26:57PM +0200, Philipp Kern wrote:
>> On Fri, Apr 20, 2012 at 10:04:28PM +0200, Philipp Kern wrote:
>> > On Thu, Apr 19, 2012 at 09:12:09PM +0200, Mehdi Dogguy wrote:
>> > > On 19/04/12 20:21, Faidon Liambotis wrote:
>> > > >Package: apt
>> > > >Version: 0.9.1
>> > > >Severity: serious
>> > > >apt 0.9.1 currently segfaults on the zelenka (our s390/s390x porterbox)
>> > > >sid_s390x chroot. Downgrading apt to 0.8.15.10 makes it work again.
>> > > Does it also segfault on s390? (s390x is not a release arch yet so
>> > > it doesn't warrant an RC severity, unless the maintainer thinks so).
>> > It breaks many package builds on s390x and it's broken on an
>> > architecture where it built before.
>> >
>> > And wrt s390x I can pull a Hurd: "The current progress is however encouraging."
>>
>> It's in the construction of the MD5SumValue.  But I'm not prepared to curse at
>> bzr for the remainder of the evening, about a bisect plugin that frankly
>> doesn't do what it's supposed to do.
>
> 2129.4.17  kalnisc |    /* Record the Description (it is not translated) */
>                   |    MD5SumValue CurMd5 = List.Description_md5();
>                   |    if (CurMd5.Value().empty() == true)
>                   |       return true;
>                   |    std::string CurLang = List.DescriptionLanguage();
>                   |
>                   |    /* Before we add a new description we first search in the group for
>                   |       a version with a description of the same MD5 - if so we reuse this
>                   |       description group instead of creating our own for this version */
>                   |    for (pkgCache::PkgIterator P = Grp.PackageList();
>                   |    P.end() == false; P = Grp.NextPkg(P))
>                   |    {
>                   |       for (pkgCache::VerIterator V = P.VersionList();
>                   |       V.end() == false; ++V)
>                   |       {
>                   |     if (IsDuplicateDescription(V.DescriptionList(), CurMd5, "") == false)
>                   |        continue;
>                   |     Ver->DescriptionList = V->DescriptionList;
>                   |     return true;
>                   |       }
>                   |    }
>
> When IsDuplicateDescription is called, calling md5() on the V.DescriptionList()
> points to unallocated memory.  Any idea why that could be?

So you mean the Description struct is invalid (V.DescriptionList()) or
the V.DescriptionList().md5() char* ?

We are missing a bit of error checking here (callers of NewDescription() do
not check if return is != 0 and IsDuplicateDescription doesn't check if the
given Description is valid), but both shouldn't be a problem as NewDescription
can only really fail if new memory can't be allocated and as each version has
at least one description you shouldn't hit a problem in the dup check either.
Both wouldn't be limited to s390x either way:
We seem to have a similar bugreport from ppc64 (#669243),
if i understand right it's also bigendian 64bit, but no other report.

The code in pkgcachegen.cc was reworked for multi-arch and especially the dup
check is new and the code as such works wih pointer left and right, but non of
it should be architecture dependent… Somehow i fear that it's more related to
our checksum changes. We had way to many problems with sha1 and sha2 to assume
md5 would be okay (the code for md5 itself is not new, but the code warping
around it).

You mentioned bisecting? Any insigns which revisions are (not) effected?
(bzr has no bisect included by default, and last time i check the plugin was…
 lets say suboptimal for us as we tend to have "big" merges)


Best regards

David Kalnischkies