[Rpm-maint] [PATCH 4/5] Extend header size to 64MB due to file signatures
Stefan Berger
stefanb at us.ibm.com
Fri Apr 29 13:31:38 UTC 2016
Lubos Kardos <lkardos at redhat.com> wrote on 04/29/2016 08:33:58 AM:
>
> Here is a backtrace for rpmfilesPopulate() called during the first
phase:
>
> rpmfilesPopulate() at rpmfi.c:1427
> rpmfilesNew() at rpmfi.c:1585
> getFiles() at rpmte.c:110
> addTE() at rpmte.c:173
> rpmteNew() at rpmte.c:241
> addPackage() at depends.c:438
> rpmtsAddInstallElementat() depends.c:493
> rpmInstall() at rp minstall.c:583
> main() at rpmqv.c:295
>
> Here is a backtrace for rpmfilesPopulate() called during the second
phase:
>
> rpmfilesPopulate() at rpmfi.c:1427
> rpmfilesNew() at rpmfi.c:1585
> getFiles() at rpmte.c:110
> rpmteOpen() at rpmte.c:571
> rpmteProcess() at rpmte.c:755
> rpmtsProcess() at transaction.c:1362
> rpmtsRun() at transaction.c:1542
> rpmcliTransaction() at rpminstall.c:289
> rpmInstall() at rpminstall.c:617
> main() at rpmqv.c:295
I am not sure whether this patch is entirely correct:
diff --git a/lib/rpmte.c b/lib/rpmte.c
index 248956c..5134e9b 100644
--- a/lib/rpmte.c
+++ b/lib/rpmte.c
@@ -88,12 +88,15 @@ void rpmteCleanDS(rpmte te)
te->order = rpmdsFree(te->order);
}
-static rpmfiles getFiles(rpmte p, Header h)
+static rpmfiles getFiles(rpmte p, Header h, int in_phase2)
{
rpmfiFlags fiflags;
fiflags = (p->type == TR_ADDED) ? (RPMFI_NOHEADER |
RPMFI_FLAGS_INSTALL) :
(RPMFI_NOHEADER |
RPMFI_FLAGS_ERASE);
+ if (!in_phase2)
+ fiflags |= RPMFI_NOFILESIGNATURES;
+
/* relocate stuff in header if necessary */
if (rpmteType(p) == TR_ADDED && rpmfsFC(p->fs) > 0) {
if (!headerIsEntry(h, RPMTAG_ORIGBASENAMES)) {
@@ -170,7 +173,7 @@ static int addTE(rpmte p, Header h, fnpyKey key,
rpmRelocation * relocs)
p->fs = rpmfsNew(rpmtdCount(&bnames), (p->type == TR_ADDED));
rpmtdFreeData(&bnames);
- p->files = getFiles(p, h);
+ p->files = getFiles(p, h, 0);
/* Packages with no files return an empty file info set, NULL is an
error */
if (p->files == NULL)
@@ -568,7 +571,7 @@ static int rpmteOpen(rpmte te, int reload_fi)
if (h != NULL) {
if (reload_fi) {
/* This can fail if we get a different, bad header from
callback */
- te->files = getFiles(te, h);
+ te->files = getFiles(te, h, 1);
rc = (te->files != NULL);
} else {
rc = 1;
rpmteOpen only has one caller, which is rpmteProcess. But rpmteProcess has
2 callers from lib/transaction.c : runTransScripts and rpmtsProcess. I
didn't follow those further.
addTE has just one caller, which is rpmteNew, which in turn has 1 possible
caller from lib/verify.c::rpmVerifyScript, which also only has one caller
from lib/verify.c::showVerifyPackage. So the '0' to getFiles is likely
correct.
Stefan
>
> One solution how to detect phase could be adding another argument to
function
> getFiles() and this argument will indicate if getFiles() was called from
> addTE() i. e. the first phase or from rpmteOpen() i. e. the second
phase then
> flags could be set according to this argument in function getFiles().
>
> Lubos
>
>
> ----- Original Message -----
> > From: "Stefan Berger" <stefanb at us.ibm.com>
> > To: "Lubos Kardos" <lkardos at redhat.com>
> > Cc: "Florian Festi" <ffesti at redhat.com>, rpm-maint at lists.rpm.org
> > Sent: Friday, April 29, 2016 1:35:35 PM
> > Subject: Re: [Rpm-maint] [PATCH 4/5] Extend header size to 64MB
> due to file signatures
> >
> > Lubos Kardos <lkardos at redhat.com> wrote on 04/29/2016 06:40:16 AM:
> >
> > >
> > > It is just a thought. Rpm transaction can be divided in two phases.
> > > In the first phase in the beginning of transaction rpm loads all
file
> > infos to
> > > perform transactions checks and then releases them. In the second
phase
> > rpm
> > > reloads single file infos to install single packages in row. The
memory
> > peak
> > > happens in the first phase when all file infos are loaded. These
file
> > infos
> > > contain also file signatures but in the first phase they needn't to
> > contain
> > > them because the signature checking is performed only in the second
> > phase.
> > >
> > > So if the file signatures blow up the file infos so much so we need
> > > to increase
> > > maximum header size then maybe it would be nice not to load file
> > signatures
> > > into file infos during the first phase of transaction when the rpm
> > memory peak
> > > happens.
> >
> > I agreed and it would be a separate patch.
> >
> > I didn't look very deeply, but how does one detect the phases? I
suppose
> > the part
> > to skip would be in lib/trpmfi.c::rpmfilePopulate where the flag
> > RPMFI_NOFILESIGNATURES is
> > checked. If that's the case, maybe the 1st phase could call this
function
> > with this
> > flag always set?
> >
> > Stefan
> >
> > >
> > > Lubos
> > >
> > > ----- Original Message -----
> > > > From: "Florian Festi" <ffesti at redhat.com>
> > > > To: "Stefan Berger" <stefanb at us.ibm.com>
> > > > Cc: rpm-maint at lists.rpm.org
> > > > Sent: Friday, April 29, 2016 10:27:39 AM
> > > > Subject: Re: [Rpm-maint] [PATCH 4/5] Extend header size to 64MB
> > > due to file signatures
> > > >
> > > > On 04/27/2016 09:47 PM, Stefan Berger wrote:
> > > > > "Rpm-maint" <rpm-maint-bounces at lists.rpm.org> wrote on
04/27/2016
> > > > > 05:50:54 AM:
> > > > >
> > > > >
> > > > >>
> > > > >> Well changing header size limit needs a bit more thought. The
main
> > > > >> problem is that packages with bigger header will look broken on
> > older
> > > > >> rpm versions and the usual way of dealing with this (adding
> > rpmlib()
> > > > >> Requires) won't work it needs reading the header.
> > > > >
> > > > > These huge headers are only occurring in a few very large
packages
> > and
> > > > > only if one applies the per-file signatures. So most users
probably
> > > > > won't notice.
> > > > >
> > > > >>
> > > > >> Also I wonder if we should increase the header size even more,
to
> > get
> > > > >> rid of this topic for a longer time. I thought about 256MB
which
> > gives a
> > > > >> 4 times increase over the 16MB. I am kinda tempted to go even
> > further.
> > > > >> Otoh the limit is there for a reason. And having rpm chew
through
> > one GB
> > > > >> of broken data doesn't sound like a pleasant experience.
> > > > >
> > > > > Anything >=16 MB works with signed files for all packages in
Fedora
> > 23.
> > > > > Let me know if you want me to resubmit the patch with a higher
> > limit.
> > > >
> > > > Yes, please. 256MB is probably the way to go. Let's hope we don't
> > reach
> > > > that any time soon.
> > > >
> > > > Florian
> > > >
> > > > --
> > > >
> > > > Red Hat GmbH, http://www.de.redhat.com/, Registered seat:
Grasbrunn,
> > > > Commercial register: Amtsgericht Muenchen, HRB 153243,
> > > > Managing Directors: Paul Argiry, Charles Cachera, Michael
Cunningham,
> > > > Michael O'Neill
> > > > _______________________________________________
> > > > Rpm-maint mailing list
> > > > Rpm-maint at lists.rpm.org
> > > > http://lists.rpm.org/mailman/listinfo/rpm-maint
> > > >
> > >
> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20160429/c0a5a828/attachment-0001.html>
More information about the Rpm-maint
mailing list