On 4 October 2017 at 17:59, Thierry Vignaud <thierry.vign...@gmail.com> wrote: >>>>>>>>> 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
One difference being that FC26 uses perl-5.24 whereas mga6 uses 5.22 > 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 _______________________________________________ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
