[Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault (was: Re: [Rpm-announce] RPM 4.14.0 release candidate 2 is out)

Wed Oct 4 15:59:27 UTC 2017

On 4 October 2017 at 14:17, Thierry Vignaud <thierry.vignaud at gmail.com> wrote:
>>>>>>>>> Also this new rpm introduced segfault regressions in both RPM4 &
>>>>>>>>> urpmi
>>>>>>>>> testsuites
>>>>>>>>> See attached gdb traces in BUG*.txt
>>>>>>>>> valgrind seems to hint about invalid writes/reads
>>>>>>>>> See you
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The urpmi issue is when checking bogus pkgs.
>>>>>>>> The RPM4 issue is when traversing the transaction (not the rpmdb)
>>>>>>>> Attached are the valgrind outputs
>>>>>>>>
>>>>>>>
>>>>>>> So we have stuff like
>>>>>>>
>>>>>>> ==14087== Invalid write of size 4
>>>>>>> ==14087==    at 0x103AA6DD: headerUnlink (header.c:188)
>>>>>>> ==14087==    by 0x103AA6DD: headerFree (header.c:194)
>>>>>>> ==14087==    by 0xFF69314: XS_RPM4__Header_DESTROY (RPM4.xs:890)
>>>>>>> ==14087==    by 0x3F512E2C40: Perl_pp_entersub (pp_hot.c:4231)
>>>>>>> ==14087==    by 0x3F5125551E: Perl_call_sv (perl.c:2848)
>>>>>>> ==14087==    by 0x3F512E7C09: S_curse (sv.c:6987)
>>>>>>> ==14087==    by 0x3F512E84F7: Perl_sv_clear (sv.c:6591)
>>>>>>> ==14087==    by 0x3F512E898D: Perl_sv_free2 (sv.c:7088)
>>>>>>> ==14087==    by 0x3F513182E6: UnknownInlinedFun (inline.h:200)
>>>>>>> ==14087==    by 0x3F513182E6: Perl_free_tmps (scope.c:212)
>>>>>>> ==14087==    by 0x3F512DAD74: Perl_pp_nextstate (pp_hot.c:52)
>>>>>>> ==14087==    by 0x3F512DAA55: Perl_runops_standard (run.c:41)
>>>>>>> ==14087==    by 0x3F5125D236: S_run_body (perl.c:2524)
>>>>>>> ==14087==    by 0x3F5125D236: perl_run (perl.c:2447)
>>>>>>> ==14087==    by 0x400C79: main (perlmain.c:123)
>>>>>>> ==14087==  Address 0xffeffef8c is on thread 1's stack
>>>>>>> ==14087==  396 bytes below stack pointer
>>>>>>>
>>>>>>> ...and all the failures are around headerFree(), but none of the
>>>>>>> traces
>>>>>>> go
>>>>>>> into rpm itself, so I dont really know what does "traversing the
>>>>>>> transaction" actually mean.
>>>>>>
>>>>>>
>>>>>>
>>>>>> in both URPM & RPM4 bindings, there's a family of traverse functions
>>>>>> that enable to execute a callback on each package of either:
>>>>>> - rpmdb
>>>>>> - the current transaction,
>>>>>> - pkgs header from an hdlist file
>>>>>> - synthetic metadata from a synthesis file
>>>>>> calling the callback on the currrent header
>>>>>>
>>>>>> See:
>>>>>> - Db_traverse*() & Trans_traverse() in perl-URPM/URPM.xs
>>>>>> - Ts_traverse() in RPM4-0.36/src/RPM4.xs
>>>>>>
>>>>>> It's loosely documented at
>>>>>> http://search.cpan.org/~tvignaud/URPM-5.12/URPM.pm or
>>>>>>
>>>>>>
>>>>>> http://search.cpan.org/~tvignaud/RPM4-0.36/lib/RPM4/Transaction.pm#Hdlist::Db-%3Etraverse_headers(sub)
>>>>>> (RPM4's doc is very incomplete -- I'm only maintaining this b/c other
>>>>>> tools depend on that and I cannot break them when upgrading rpm)
>>>>>>
>>>>>>> But the problem is simply with perl-RPM4 and
>>>>>>> urpmi passing uninitialized variables to headerFree().
>>>>>>>
>>>>>>> What changed in rpm is that rpmReadPackageFile() no longer does this
>>>>>>> as
>>>>>>> the
>>>>>>> first thing:
>>>>>>>
>>>>>>>       if (hdrp)
>>>>>>>           *hdrp = NULL;
>>>>>>>
>>>>>>> Ie if you pass an uninitialized pointer as hdrp, it remains
>>>>>>> uninitialized
>>>>>>> unless rpmReadPackageFile() returns with a success code.
>>>>>>> Which is how I think it should be, but it does deserve a release note
>>>>>>> on
>>>>>>> the
>>>>>>> changed API.
>>>>>>>
>>>>>>> So the moral of the story is basically: if you depend on your
>>>>>>> variables
>>>>>>> being initialized, initialize them by yourself. It's a good practise
>>>>>>> anyway.
>>>>>>
>>>>>>
>>>>>>
>>>>>> I'll check for uninitialized variables later today
>>>>>
>>>>>
>>>>>
>>>>> Actually the bug happen when running the transaction
>>>>>
>>>>
>>>> Right, that makes sense, there's a second rpmReadPackageFile() inside
>>>> transaction run.
>>>
>>>
>>> Intestingly, perl-RPM4 no more segfaults (there still faillures
>>> though) if I alter the transrun() method (which calls rpmtsRun in
>>> rpmlib) 's perl callback to *not* call std rpmShowProgress())
>>> See attached patch
>>>
>>> Aka previously it runs both the passed perl callback + rpmlib default
>>> callback which worked smoothly.
>>> But since rpm-4.14, calling std rpm callback results a segfault...
>>> I'm happy to not call that and I don't think we have any tool actually
>>> running transactions^W installing packages through RPM4 right now but
>>> still that's a regression in rpm...
>>> Maybe some new unlink call or sg like that
>>>
>>
>> Are you saying you still get that segfault with this fix?
>> https://github.com/rpm-software-management/rpm/commit/082e6e77dd613efa643b02ee0417c1382f520893
>
> That fix was for URPM segfaulting while verifying bad rpms, here I'm
> talking about RPM4 segfaulting while installing packages
> (different binding, different issue)
> The URPM issue is fixed by the above commit (s/is/should/ - I hadn't
> actually tested your fix)
> The RPM4 issue is a different issue, even if both happen in headerFree() .
> You might get confused by the perl gdb traces that looks similar but
> the issues really are different.
> They show the point where the perl interpreter segfault, but not the
> point where the call in rpmlib has garbaged things as this action has
> finished and we'd return to the perl interpreter.
>
>> From the traceback you posted it was tripping on the uninitialized header
>> pointer as the other one. And if you disable the entire callback then there
>> are no open files returned and the whole error path is different.
>
> There's still a perl callback that's run.
> Actually there's a C/XS wrapper (transCallback()), running the
> optional pure perl callback if present
> Ts_transrun => rpmtsSetNotifyCallback with transCallback wrapper as
> callback and the actual perl callback as parameter
> (similar to what URPM is doing)
> Then during the actual installation, rpmlib calls the callback
> (transCallback) which set up the perl arg stack & calls the perl
> callback which itself used to call the rpmlib default callback.
>
> Here, the segfault is also in headerFree() but only happen in the
> install callback while installing pkgs and only if the perl callback
> calls the default rpmlib progress callback.
>
>> BTW, your workaround in
>> http://gitweb.mageia.org/software/rpm/perl-URPM/commit/?id=87dbde4f3b078173e53cd45cac000c2d2751b370
>> is not the one you want: initialize to NULL instead
>
> Ack. Thx
> Though from what you said, I should be able to remove any
> initialization at all as the rpmReadPackageFile() regression is fixed
> and should now always set the return parameter.
> See you

You can test by building the perl-RPM4 packages with the attached spec
file, w/o installing it.
Sources are at http://search.cpan.org/CPAN/authors/id/T/TV/TVIGNAUD/RPM4-0.36.tar.gz
then in order to run the broken test with the just compiled module:
cd BUILD/RPM4-0.36/
PERL_DL_NONLAZY=1 PERL_USE_UNSAFE_INC=1 gdb -q --args perl
"-MExtUtils::Command::MM" "-MTest::Harness" -Iblib/lib -Iblib/arch
t/05transaction.t
What's interesting is that:
- it segfaults on FC26 with rpm-4.13.0.1-7.fc26.x86_64
- it doesn't segfaults but show failures on mga6 with rpm-4.13.0.1-3.1.mga6

Some patch difference could explain that
I see that it hasn't been rebuild for rpm updates between
4.13.0-0.rc1.32.mga6 & 4.13.0.1-3.mga6 and I guess the other
regressions weren't checked for after a signing segfault in the
testsuite due to rpm-4.13 was fixed
-------------- next part --------------
A non-text attachment was scrubbed...
Name: perl-RPM4.spec
Type: text/x-rpm-spec
Size: 1629 bytes
Desc: not available
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20171004/3ff6336d/attachment.bin>