On 10/04/2017 01:25 PM, Thierry Vignaud wrote:
On 3 October 2017 at 09:12, Panu Matilainen <pmati...@redhat.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

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.

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.

        - Panu -
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to