- When on-disk %config file contents match the contents of the new
package (such as config in old package was buggy, fixed in new
and admin worked around in the meantime) there's no point creating
a backup that's identical to what you already have. Note that
we create the new config anyway instead of skipping as timestamp
might have changed.
- Adjust test-cases to the new saner behavior.
- When packages share an identical file, we only need to create the
first one we encounter and skip it for the rest (reverse already
happens, and must happen, on erase). This has the benefit of
getting the timestamp to match the first file, which often is
also the last one to get removed. Also when lots of shared files
are involved (such as in multilib installations), this can give
several percents worth of speed gain as we dont unnecessarily rewrite
the same contents over and over.
- Commit 429b933b42 fixes the fd-leak
regression in rpm 4.10, but we already have code which relies
on the new behavior and segfaults with the former one. Adjust
rpmcpioClose() to the old behavior to get master tree back to
working shape. TODO: fix this properly some rainy day (oh and see
http://geekandpoke.typepad.com/geekandpoke/2012/08/likelihood.html)
- Verifying files against the originating, non-installed package can
be useful sometimes, eg if rpmdb is broken or cannot be trusted.
Of course such verification will give false positives on files that
were intentionally skipped during installation (nodocs, wrong color,
netshared...), that can't be helped.
- This restores the former behavior that was erroneously changed
in commit adbd484372 while "fixing"
an unhandled enumeration in the switch. Doh.
- As noted (but since then blissfully forgotten) in the commit message,
commit e696b409fe broke --badreloc
so its been broken since rpm >= 4.9.x :-/
- Transaction problem filter is only available during rpmtsRun() so
we have no clue whether bad relocations should be filtered or not
during rpmte creation. Instead of creating the problems at rpmteNew()
time, remember any bad relocations that were found and check +
create the actual problems (unless filtered) along with other
problems during rpmtsRun().
- Passing an "allow bad relocations" flag to rpmtsAddInstallElement()
would be a saner option but this is a back-portable way of handling it.
- For now there's no way to set this flag from packages so it doesn't
actually do anything, but it doesn't hurt anything either, allows
Suse & friends to drop one patch (greetings Michael :) and there
are might be some things we could use this for internally.
- At the time when the file list is being processed, we dont yet
have the slightest clue what kind of payload will be used for
for the archive or what limits it might have. Let the cpio code
handle its own limits checking, the build-side only needs to
worry about whether 32bit uints are sufficient for storing the
sizes in headers.
- Commit 4cb02aa928 asked to see
what breaks when mmap() is used, now we know: large package support
broke when enabling it. Could be fixed of course by eg adding
a size cap to the fsm part as well, but just doesn't seem worth it:
I fail to measure any meaningful performance improvement from mmap
usage in either case, and added complexity for what is close to
zero benefit just doesn't make sense... and various sources in fact
note the rpm usage (read through the entire file sequentially) as one
of the cases where mmap() is NOT beneficial due to mmap() high
setup + teardown cost + page fault speed (or lack of thereof).
- Refer to RPMTAG_* for rpmds tags, not RPMDBI_*. The values for these
are the same, only the "intended use" differs (RPMDBI refers to
rpmdb indexes) so this doesn't change anything in practise.
- dbtag cannot possibly be on obsoletes here, we need to look at deptag
instead. This thinko in commit 5f1ec21518
renders that whole commit useless. Doh.
- RPMFILE_EXCLUDE only exists during spec parse, and doesn't "leak"
into headers only because the file is, well, excluded to start with.
Unexport the internal-only bit and explicitly strip out any excess
bits from data going to header. The current 16/16 split is artificial
of course, RPMTAG_FILEATTRS is 32bit so there's plenty of room
for growing new file attributes, with internal-only adjustments
required.
- Eliminate RPMFILE_UNPATCHED while at it, this is a leftover
from Suse patch rpms which are no longer used anywhere.
Hi Panu et al,
Here's a small patch that changes the ordering used for putting
hardlinked files into the cpio archive back to lexicographical.
You might wonder what this is about. Well, old rpm-3 (and
also old versions of rpm-4, I think) already used lexicographical
ordering for files and hardlinks. When deltarpm was created,
it made use of this fact when "compressing" the file order
of the cpio archive into the so-called "sequence". Deltarpm
can deal with "out of order" files, but in that case it needs
to reset the compression, which leads to really long sequence
strings.
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
- BDB wants to use mmap() for its environment by default, but not
all (file)systems support this, as pointed out by Daniel Drak.
However env->open() can return EINVAL for a number of reasons,
require all the fallback reasons to be system level errors to
differentiate from "logical" errors such as incompatible flags
to (possibly pre-existing) db environment, in which case we better
just error out.
- The only remaining use for rpmts here was grabbing NODIGESTS
from rpmtsFlags(). Pass the tiny little piece of information
as an argument for the one place needing it and rip all the
now unused related goo.
- Most of fsm doesnt need the actual transaction set for anything "real"
and dragging it around as a mere statistics collector seems
pretty dumb. If we want better statistics, we better come up with
a saner way to gather them.
- Package building has no associated transaction or ts members,
this was all just fake-up kludgery to work around the way how
fsm used to work. None of it relevant now, kill kill kill.
- Determine the need for reverse iteration based on fsm goal
- Everything else was just using rpmte to get to its file states,
eliminate the intermediate ping-pong by passign the file states
around directly. Makes the thing that little bit less silly.
- Ghosts are never backed up, and the whole business is irrelevant
for package building. Use fsm goal instead of rpmte mode to
determine what to do, rpmte in build code is nothing but an ugly hack.
- rpmfs is such a low-level construct it doesn't need to know anything
about the upper layers. Gather the necessary bits of info in the
sole caller instead and pass only whats needed to rpmfsNew() to
enable creating a filestate item without having rpmte/header at hand,
which we'll be needing in the fsm shortly.
- This whole thing probably isn't needed anymore, but for now just
lift the FA_SKIP/FA_COPYOUT setting to rpmPackageFilesArchive(),
allowing rpmfsSetAction() to become properly internal-only function.
- Trim out unnecessary now unnecessary librpm internal includes
from librpmbuild code
- This allows much nicer handling some common scenarios such as
upstream pre-releases where the pre-release version would normally
appear newer than final release, eg 1.0-rc1 vs 1.0. Previously this
required mapping the pre-release tag into the release tag to achieve
desired sorting, with tild this becomes simply 1.0~rc1 < 1.0.
- Add a rpmlib() tracking dependency to prevent older rpm versions
from getting confused with packages relying on the new behavior.
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
- Fix oversight in commit 9fb81eac0bcd239e46459f72916099f1adfc5cb9:
rpmdsMatchesDep() works on provides, but obsoletes are matched on
package name so we want rpmdsNVRMatchesDep(). rpmdsMatchesDep()
"worked" as the implicit self-provide is always at the 0 index
on packages created by modern rpmbuild, but this isn't really
guaranteed: very old V3 rpms can have something else at the
first index, and ditto for non-rpmbuild created packages.
- Thanks to Michael Schroeder for pointing this out.
- Now that there are no more failing parts requiring return codes,
change + rename fsmSetup() into a more regular fsmNew() construct,
returning newly "instance" of fsm and similarly fsmTeardown() ->
fsmFree() to free the thing.
- There's no real need to allocate this stuff on heap, but doing
so makes life actually simpler for the three callers and makes the
whole thing more consistent with common practises in the codebase.
- Checking missing links is only relevant for install, lift the
code to separate function and call from the install-case only.
- Freeing data while checking seems like a dubious "optimization"
but to keep changes minimal, leaving that as it is now.
- When writing, archive finalization can write further data into
the payload (eg cpio trailer). We need to be able to fish the
final archive size *after* closing it, otherwise archive size
will be off by trailer size.
- Add new rpmcpioFree() function which simpy frees the archive struct,
calling rpmcpioClose() in the process if it wasn't already done.
- This also simplifies the error code gymnastics in fsm: we need to
free the resources whether error or not, which rpmcpioFree() allows
us to do. And for writers, calling rpmcpioClose() only needs to
be done if earlier parts were successfull, so we dont need to worry
about masking a former error code when calling it.
- Only install and build have an associated archive, and this can
be just as well handled as a local variable, passing down as
argument to the handful of places needing it.
- Error handling looks a bit fishy but that's not exactly new issue...
- The mapping index is the same for build + erase, but different
for install and so is the return code mapping. By doing this
inline where the action is we avoid having to fiddle with
mapped return codes for the exit case.
- Besides simplifying the common path everything takes, this removes
the need to map error returns back and forth when we can just
detect the end of payload directly in the loop and break out.
- There's zero need for the rest of the code to know or care about
dnl iterator, just pass the file info- and state sets to
fsmMkdirs() directly and hide the otherwise unused iterator
business there.
- Besides being a bizarre name for "some extra initialization" work,
fsmCreate() was full of redundant goo, including a rather complex
way of mapping a zero return back to zero in case it really was
zero .. or something.
- rpmPackageFilesInstall() is the only case where this needs to occur,
move it there instead of yet-another goal-dependent piece where it has
no business being in the strangely named fsmCreate()...
- Except for expandRegular(), all notification occurs from the three
main worker functions. Pass as an argument for the lone special
case, the other noticy already have the psm as their own argument.
- fsmCommit() does not advance the archive or anything else, so
this was simply issuing the same exact notification that we
just did from rpmPackageFilesInstall().
- Only handle %missingok in the cases where it actually applies,
and additionally handle missing %ghost which is not an error
either. Dont log anything for these non-errors.
- Unify the error handling for files and directories, makes life
simpler as they dont differ by that much.
- Log real failures as warnings instead of silencing them to debug
spew, users will want to know if something that was supposed to
be removed was not (say, a file with immutable attr set).
- Add comments for further work on this area.
- In their previous life these breaks applied to a switch-case but
now they end up aborting the loop on first successful removal, causing
everything but the first file/directory of a package to be left behind
on erase. Fixes the previously unnoticed regression from commit
1845c1d711.
- For practical purposes, the "content" of a device node is its
minor+major number, if those differ the files are very much
not the same and thus cannot be shared.
- Two files (or directories) cannot be correctly shared if their
permissions differ, even if the content is identical: either
file will end up having wrong permissions, depending on installation
order. This means a package can among other things silently
eg relax permissions of eg security sensitive directory (accidentally
or intentionallY).
- We now require exact match of user, group and entire file mode
(previously only the file type part of mode was tested)
- Packages having file conflicts with itself may seem absurd, but
directory symlinks (such as /lib being a symlink to /usr/lib)
make this entirely possible. This makes us catch and abort early
on these cases instead of silently overwriting the self-conflicting
files, potentially with disastrous results.
- Packages having file conflicts with itself may seem absurd, but
directory symlinks (such as /lib being a symlink to /usr/lib)
make this entirely possible. This makes us catch and abort early
on these cases instead of silently overwriting the self-conflicting
files, potentially with disastrous results.
- The final pre-requisite to handling file conflicts within a package:
with this we're no longer tied to the single index per rpmfi. This
is not supposed to change anything yet unless I screwed something up.
Also goes to show that a semi-iterator interface for something
that really needs random access only gets in the way rather than
helping...
- Further preliminaries to handle file conflicts within a package.
- These are internal-only interfaces so we can just change without
bothering with compat wrappers.
- Preliminaries for handling file conflicts within a package:
Using rpmfi's self-iterator limits access to the file info to
one caller at a time, in order to self-file conflicts we'll need
to be able to access the same rpmfi at different indexes simultaneously.
- As these are public API's, add compat wrappers for the self-iterator
use (although AFAIK nothing except rpm itself uses these)
- Turn the strange negated condition around: when dealing with
config files test for it directly. This way, all the special
cases are handled first and normal cases fall through the if-jungle
to exactly one case of rpmfsSetAction(fs, i, FA_ERASE). Makes
the logic more obvious, at least to me.
- handleOverlappedFiles() only needs the file name in a couple
of relatively rare special cases. Constructing the fn more
expensive than other rpmfi-calls, dont bother unless actually needed.
- Previously this would return a pointer to an internal per-rpmfi buffer
whose contents get silently overwritten on each call to rpmfiFNIndex(),
making it unsafe for unsafe for random access for more than one
active caller (such code does not currently exist in rpm though)
- Make rpmfiFNIndex() always return freshly allocated memory, and adjust
the rpmfiFN() iteration wrapper to free and realloc the internal
"buffer" on each call. It's a wee bit slower than before but it's
not called *that* much, and if needed there are ways to optimize it.
- Similar to commit 9fb81eac0b but
on the to-be-installed set: obsoletes should only be matched against
package names, not any provide or file names. Hasn't really mattered
previously due to the way its called, but since commit
05487d9a3f I guess it started to matter.
It's more correct this way anyhow, and should fix RhBug:810077.
- Since rpmal only knows about provides, we need to handle obsoletes
as a special case and filter out matches on provide names different
than the matching package name.
- Undo the ancient broken fix for RhBug:71996 from commit
9e06e3b8ca76ae55eaf2c4e37ba9cac729789014: instead of disabling
the check, pass in the correct upper range which is entirely
different from everything else for the region trailer tag.
- Fixes CVE-2012-0815
- Non-existent region tag is very different from existing but invalid
one - the former is not an error but the latter one is, and needs
to be handled as such. Previously an invalid region tag would cause
us to treat it like rpm v3 package on entry, skipping all the region
sanity checks and then crashing and burning later on when the immutable
tag is fetched.
- Refer to REGION_TAG_TYPE instead of RPM_BIN_TYPE wrt the expected
type of region tag for consistency and clarity, they are the same
exact thing though.
- Should unify these damn copy-slop check one of these days, sigh...
For now, settling for the easily backportable approach.
- Fixes the other half of CVE-2012-0060
- Region tags need to have very specific content, the generic
header tag checks are not sufficient to ensure sanity. Verify
the tag is one of the known region tags and that the entry has
expected type and count.
- Fixes the first half of CVE-2012-0060
Move cleaning the stat_s structs to fsmInit() (beginning of the loop instead of the end)
Drop freeing fsm->path as this is done in fsmInit() and fsmTeardown()
- For example, warn when building an x86_64 package that only contains 32
bit binaries.
- This should indicate to the maintainers that they might have gotten the
architecture wrong.
- Introduces 'archcolor' in rpmrc so we can map architectures to colors.
- Related: RhBug:713323
Fseek() does not return a proper error code. This needs to be fixed before we can use it as most comprssed files do not support seeking and we need to be able to detect this reliably
Create cpio_t data type that holds the underlaying FD_t instance
Move padding and position handling and trailer generation into cpio.c
Use only one buffer in the fsm (merging the read and write buffer)
Replace the FSM_EAT, FSM_POS, FSM_PAD, FSM_DREAD, FSM_DWRITE states with cpio functions
Prepend "rpm" to the cpio function names
- rpmdb_dump, load, recover, verify, stat etc are useful at times,
but these are not. This also fixes build with internal db for
more recent versions of Berkeley DB.
- If the caller doesn't know the end pointer, we dont have a whole lot
of chance to come up with a reasonable one either. Just assume
the terminating \0's are there when end boundary is not specified:
when this happens we're dealing with relatively "trusted" data
anyway, the more critical case of reading in unknown headers does
always pass end pointers.
- While capping the end pointer to HEADER_DATA_MAX seems like a
reasonable thing to do (as was done in commit
f79909d04e), it doesn't really help
(bad data would likely run past bounds anyway), and it's not right
either: the pointer can be to a stack address, and the stack can be
near the top of addressable range, and ptr + HEADER_DATA_MAX can
cause pointer wraparound. Notably that's exactly what happens
when running 32bit personality process on 64bit system on Linux,
at least in case of i386 process on x86_64, causing all sorts of
breakage..
- rpmteClose() will wipe out the file info to free memory, we only
should care whether we failed to (re)load the file info. This
thinko in commit 06a2f1269b
broke %posttrans scriptlets (and without commit
274dbf557d, %pretrans in other
circumstances), whoopsie *blush*. Now, off to write a test-case
for our scriptlet behavior...
- Despite commit cef18c9480, we'd still
end up freeing the file info sets via rpmteClose() while going
through the test-transaction packages. This together with commit
06a2f1269b caused install failures
on packages which have %pretrans scriptlets, if a test-transaction
was first performed on the same transaction set that gets used
for the "real" transaction as well. How wonderfully obscure...
- This is stupid... only librpm and librpmio actually need the bump due
to ABI breakage, librpmbuild and librpmsign are unchanged and could
use just a revision bump. But just incrementing the revision (or age)
would set us on collision course with maintenance updates to 4.9.x.
Then again its not like you can actually use librpmbuild or librpmsign
without also linking to librpm(io) so from everything needs rebuilding
anyway. This all also pretty much makes the whole libtool library
versioning a bit moot. Bah.
- "pure install" like 'rpm -i' never removes anything (long-standing
behavior that we dont want to change), but that causes us to allow
installing obsoleting packages without removing what they obsolete,
which in turn causes errors on verify. Not good.
- This (together with commit 9fb81eac0b)
makes obsoletes behave like conflicts in such a case, preventing
the inconsistency from taking place. Also verify will now whine
on all the involved packages on inconsistencies.
- Obsoletes should only be matched against package names, not provides,
or file names for that matter. This hasn't really mattered so far
due to the way rpmdbProvides() gets called currently, but there's
a missing case that requires this...
- For non-hashed mode, dump the full NEVRA string. This might break
programs that are parsing the --percent format, but such programs
almost certainly need adjusting for the added erasure progress anyway.
- For hashed mode, show package NEVR instead of just name. Otherwise
somebody will sooner or later file a bug on "confusing output"
as it might seem it's removing what it just installed in upgrade-mode.
Full NEVRA would be better still but screen estate is tight as it is...
- Also get rid of headerFormat() call here, use RPMTAG_NEVR(A) extensions
instead.
- Presumably the rpmrc internals still need the magic table tennis
but nobody else should want to mess with this, reconfiguration
for another (build) arch is done through rpmReadConfigFiles().
- The dsi retrieved from rpmtsDbDSI() would become invalid whenever
ts->dsi structure got realloced, ie anytime when disk space is
calculated for a different fs than where the rpmdb resides. This
is likely to be the real issue behind RhBug:766260 and also RhBug:671056.
Just call rpmtsUpdateDSI() directly with suitable arguments for the
rpmdb to avoid the special (re)alloc paths.
- In addition, consider the db growth for packages with no files too.
- Commit ac0ab016a5 unintentionally
changed the order of the problems shown in verify strings due to
a dumb oversight (greetings to self, duh). In other words, this
fixes a verify output regression in rpm >= 4.9.x by restoring
the long-standing (and documented) order of the verify output chars.
- Also fix the testcase which unfortunately was only added after
the output-changing commit so it didn't catch the breakage either :-/
- Basename and dirindex counts must be equal, dirnames count must be
larger than zero and no larger than number of basenames. Check
that directory indexes are within range. Additionally file states
array size, if used, must equal to the genaral file count.
- Packages with no RPMTAG_FILESTATES cannot have installed files
from our POV, just exit early in that case.
- Rerrange the exit path a bit and only call rpmtdFreeData() on
file states if the corresponding headerGet() call was actually
made. Wont make any difference yet, but once we have other jumps
to exit it will.
- Rename rpmfiBuildFNames() to fnTag() and push all the td manipulation
there, making all the various different fn-related tags differ by
just the arguments to fnTag() and allowing central error etc
handling. No functional changes (yet) though.
- This should mostly be a can't happen case, but at least in theory
region retrieval could fail. So could unknown data type, but a header
with unknown data types shouldn't even load... Anyway, there could
be further error cases we might be able to flag here.
- Tag retrieval can in some cases fail, especially so for tag
extensions. The headerGet() interface doesn't directly allow
distinguishing between non-existent tag and existent but invalid,
but we can sneak in that information through in the tag data flags.
- 64bit inode numbers lose their uniquity when brutally truncated
to 32bit integers as we've done so far. This can and will cause rpm
(and cpio) to mix up arbitrary file entries as hardlinks and vice versa.
- As the only interesting aspect of inode numbers is whether they're
equal to something else or not, we dont have to carry the "physical"
on-disk value to preserve semantics. So we can just remap the
inode numbers to something that fits our 32bit integer tags
without causing compatibility complexies with older rpms and
cpio (and since we can't handle more than INT32_MAX files in a package
anyway, breaking compatibility for this would be just braindead dumb).
An extremely simple way to achieve this is to use our
build-time file list index as the basis of stored inode number.
- In theory this breaks inode-device pairing, but as the buildroot
cannot span across filesystems in any remotely normal conditions,
just add a sanity check to catch the dirty tricksters...
- Based on a patch by Zdenek Pavlas, just further simplified and
buildroot fs boundary sanity check added.
- There are some situations where switching keyrings might be wanted,
especially as long as we dont export a way to pass keyring as
an argument to package reading/verification functions. Since thereäs
no technical reason to disallow it, might as well allow it...
- One of the more common reasons for users to do --rebuilddb is
a paniced environment. Throwing DB_RUNRECOVER errors at the user
who is trying to recover by rebuilding the db isn't terribly
productive (RhBug:590710). Use a private environment while
rebuilding for both the original and new database, and dont bother
with CDB which only slows things down when there are no other
players present.
- Verify wants the same flags but for different reasons...
- Loading the pubkeys from database numerous often unwanted side-effects,
if signature checking is disabled then there's no point loading
the keys either.
- Commit cad147070e did this for
rpmReadPackageFile() specifically but we really want it honored
for all operations including headerCheck() and friends, handle
it centrally in loadKeys() for simplicity.
- We'd like to get rid of the potentially huge amounts of memory
eaten by file info sets as early as possible, but when there's a
chance that we'll get called again with either added transacation
elements or on-disk changes, such as %pretrans changing something
underneath us, we need to (be able to) recalculate everything
from scratch. Only free up the memory when we know we dont need
it anymore, ie on an actual transaction run.
- This doesn't change anything for rpm itself, for yum and others
which do a separate test-transaction first, it means %pretrans
directory<->symlink replacement hacks and the like have a chance
of working again. I'm sure there's a bug filed on this somewhere but...
- rpmReadHeader() performs far more initial sanity checks on the header
than headerRead() does, and makes behavior consistent with eg query
and install paths. As an added bonus we'll get more detailed
error messages too.
- Basename and dirindex counts must be equal, dirnames count must be
larger than zero and no larger than number of basenames. Check
that directory indexes are within range.
- There are mountains of further checks to be added here (and elsewhere)
but we gotta start somewhere... and filename triplets are one of the more
critical elements we got.
- Add NULL checks and add/adjust comments where appropriate.
- The remaining callers should handle NULL fi gracefully if not
entirely correctly: rpmfiFC() returns 0 on NULL fi, so these
callers just see the erronous file info set as "no files" case.
Something to fine-tune later...
- Verify that a header at least has the very basic elements like
name, version, release, os and arch (except for gpg-pubkeys which
dont have the latter two, sigh), fail if not.
- rpmfiNew() cannot currently fail but handling this error will allow
sanity checking the file metadata which can be inconsistent even if
a header is "physically" consistent.
- We'll eventually want to have sanity checks on dependency sets too, but
unlike rpmfiNew(), rpmdsNew() currently returns NULL for non-existent
dependencies (eg most packages do not have conflicts or obsoletes) to
save memory. Either that needs to change or we'll need to check
for tag existence for the meaning of a returned NULL here.
- Return error from verifyscript if rpmteNew() fails. This can't
currently happen but handling this error makes it possible to
do sanity checks on the header contents, such as file list integrity etc.
Unlikely to occur for installed packages, but verify can be run
on non-installed packages as well, where failure is more of a possibility.
- Return error from rpmtsAddInstallElement() if rpmteNew() fails. This
can't currently happen, but handling this error makes it possible
to do sanity checks on the header contents, such as file list integrity.
- If we can't open the rpmdb then we cannot correctly process the
upgrade, error out early. Mostly a "can't happen" case though.
Also makes the logic a bit clearer, hopefully.
- Return error from removePackage() if rpmteNew() fails. This can't
currently happen and is unlikely anyway on already installed
packages (this is more interesting for added packages) but
just in case...
- Handling failure from upgrade- and obsoletes erasures is trickier
both can add any number of erasure elements, and if one of them
fails we'd need to undo all the erasures caused by this element.
Just add a reminder comment for now.
- These are nothing but unnecessarily specialized Fopen(), Fwrite() and
Fclose() for what is a purely local need in expandRegular(). Move
the local stuff where it belongs.
- These are nothing but unnecessarily specialized Fopen(), Fread() and
Fclose() for what is a purely local need in writeFile(). Move
the local stuff where it belongs.
- Instead of behind-the-scenes pointer updating, use fsm->wrbuf
explicitly for the link target. Doesn't make it less hackish
but at least it now stands out.
- opath is the file that links will be made to, we grab it at the
start and free at the end. No need to save and restore what we
dont modify, one more fsm->opath usage down...
- Hopefully this makes the actual operation stand out more clearly
with the unnecessary fsm->opath fiddling out of the picture,
other than that there's not much to gain here.
- Makes the code much more obvious to follow since we're not swapping
path/opath back and forwards just to be able to use a throwaway
path for the rename. Dont bother null-checking on logging,
if one of the paths was null we'd be dead already.
- Also fixes an ancient memleak: when osuffix is in use, fsm->path
gets newly malloced before fsmVerify() but this part did another
allocation on it, didn't save and restore fsm->path .. and nothing
was freeing the original (local) allocation of fsm->path, only
restoring the previous value.
- Now that we can directly operate on temporarily variables instead
of having to save-ping-restore-pong them, lets do so...
- Also eliminating NULL-checks on the path variables in logging - if
either of the paths were NULL we would've already crashed in rename()
- Introduced in commit d15bf56a70 and
uncaught prior to push as none of the test-suite cases involve
symlinks. This is what happens when trying to quickly rewrite
git history to put combine a forgotten change into earlier ones,
guilty as charged :(
- Once all the other cruft has been carved out, turns out this needs
nothing but a directory iterator which we can init and free
in the caller easily enough, and selabel handle (duh). All the
rest is independent of fsm internals in reality.
- At the point where this runs there's probably nothing at all to
save and restore in the stat buf anyway, but it's just stupid to
abuse that when all we need is a local mode_t temp variable. Fix
it now that we can.
- This doesn't need access to the entire fsm, it just needs a buffer
to place the results in / return errors. Currently the "out" buffer
is (ab)used for the results, this just forces that to stand out
and should make it easier to sanitize later.
- This doesn't need access to the entire fsm, just path and mtime
from coming from a header originally. Will allow eliminating
save -> abuse -> restore behavior in caller but leaving till later...
- None of these needs access to the entire fsm, they only
exist to map and filter errors to rpm special needs and to
create debug swew on top of the plain syscalls.
- This doesn't need access to the entire fsm, it only exists to
map errors to CPIOERR_* and create debug foo. This will allow
eliminating save -> abuse -> restore behavior in callers, but
leaving that till later.
- This doesn't need access to the entire fsm, just regular stat()
args and a flag whether to use lstat() or stat(). "followlinks"
or such would be saner name for the flag but leaving that for now...
- This doesn't need access to the entire fsm, just regular rename()
args and a flags to see whether "secure" delete should be done.
In itself this only looks like more trouble, but all the callers
are fiddling and saving and restoring with fsm->[o]path just
to call this, which we can now avoid. Leaving sanitizing the
callers till later though, this is a minefield...
- Move parsing, setting and freeing of capabilities into simple
helper function, there's no point whatsoever having the current
capability stored in fsm when it only complicates freeing and all.
WTH was I thinking when implementing this? (well, everything in
fsm was done that way so... but that's a lame excuse)
- Move all the label foobar into a simple helper function which
finds, sets and frees the context if selinux is enabled, use
for both regular operation and orphan directory labeling.
Simplifies things a good deal...
- While the selabel handle can change during a transaction, it
wont change while the fsm is running so its sufficient to grab
it on entry instead of repeatedly calling rpmtsSELabelHandle() after
figuring out where in the world our ts might be.
- This bogosity goes back to commit fcf6b50378
which was supposed to fix files getting erased if time() returns
errors (RhBug:223931). Problem is, this "fix" didn't fix anything
at all as the suffix still wouldn't be created in the error
case, even if the FSM_UNDO part did the right thing. We always
want a suffix on installs, it doesn't matter *what* the suffix is.
- These are not used or needed outside fsmMkdirs() so its just
plain dumb to have them in the big struct. No functional changes,
just taming the fsm monster a little bit.
- Similar in spirit to PSM blackbox treatment in
commit df9cdb1321, except that
technically fsm guts are still wide-open in fsm.h due to cpio
"needing" them (yuck).
- Allows getting rid of dumb a**-backwards things like rpmfiFSM()
which is just not needed, fsm is a relatively short-lived entity
inside psm and build, nobody else needs to bother with it except
for the returned results.
- Figure out the cpio map flags in fsmSetup() where it logically belongs,
we have all the necessary info available there.
- Get rid of newFSM() and freeFSM(), we can just as well place the
fsm on stack, merge the necessary cleanup bits from freeFSM()
into fsmTeardown()
- Supposedly no functional changes, knock wood.
- rpmfi itself doesn't need it for anything, its only really used
for progress reporting during install. Grab the size into psm
total directly, this is already passed down to fsm.
- Removes one of the last remaining rpmfi opacity violations, just
fi->apath to go...
- Eliminate feeble heuristic on archive size tag not being set during
build for detecting this and have build code explicitly pass
RPMFI_ISBUILD flag instead.
- Also eliminate the pointless isSource variable from rpmfiNew() while.
- rpmpsmStage() runs with RPMRC_OK as assumed result, no need to
set explicitly here.
- Dont bother testing for justdb flag here. The justdb case doesn't
need fi->apath but it doesn't exactly hurt either, and we'll want
to eliminate the apath kludge sooner or later anyway.
- Just use the same bits as install does, they behave the same for
our purposes. On upgrades the output would get confusing especially
with --hash when package versions aren't output'ed, so we print
out extra output when entering install and erase stages. Only
do this when --hash is used (ie a human is probably watching us)
to hopefully avoid breaking scripts (including our test-suite) that
rely on the ages old behavior of non-hashed output.
- Whereas on install the progress is measured by bytes written to
disk vs total archive size, on erase the best we can do is
going by the number of files in the package. Fsm iterates backwards
on erase so we can't just use fsm->ix but need to re-revert the value.
- Move notifications from fsm to psm side for sanity and symmetry,
psm already has members to hold the callback state.
- Replace PSM_NOTIFY "state" with a helper function that both
fsm and psm itself use (except for error callbacks which are
a bit different)
- Init psm->total early, this doesn't change and can now be
used to refer to "all done" value whatever it happens to be,
instead of magic "100" values etc.
- Packages with no files are now handled through the same path
as everything else from progress reporting pov, we just skip calls
to fsm if there are no files.
- Issue stop callbacks for install as well. While INST_CLOSE_FILE
can be (and is currently) used to detect this condition, its
conceptually an entirely different thing.
- Fix erasure callback parameters, they were reversed (starting from
total and ending with 0, ehh...)
- On error we're already on our way out of the psm, no point mucking
with the psm state. No functional changes, just makes the code
a little bit shorter.
- Adds two new transaction callbacks: RPMCALLBACK_SCRIPT_START and
RPMCALLBACK_SCRIPT_STOP which get issued for every scriptlet we run.
- On script start, callback can optionally return an FD which will
override transaction-wide script fd to make it easier to accurately
collect per-scriptlet output (eg per-scriptlet temporary file).
Callback is also responsible for closing the fd if it returns one.
- For both callbacks, "amount" holds the script tag number. On stop
callback, "total" holds the scriptlet exit status mapped into
OK/NOTFOUND/FAIL for success/non-fatal/fatal errors. Abusing "notfound"
for warning result is ugly but differentiating it from the other
cases allows callers to ignore SCRIPT_ERROR if they choose to
implement stop and start.
- rpmcliHashes*, and rpmcliProgress* and rpmcliPackagesTotal are
implementation details of rpmShowProgress() and are useless outside
of it. Make them static, these shouldn't have been exported to
begin with.
- The total number of packages equals transaction order count, which
is passed as total to transaction start callback. In particular
messing with this from rpmtsAddInstallElement() is just stupid.
- This will break callers that are relying on rpmcliPackagesTotal value
outside a running transaction, but that's just stupid anyway. The
correct way to get number of elements in transaction set is calling
rpmtsNElements(), which has been there for a good part of a decade.
This enables package maintainers to:
- Force removal of a no longer supported multilib library (the patch also
removes the check against obsoleting packages of the same name).
- Deprecate packages of different header color than the package's. Note:
even x86_64 packages can have header color 1 in which case we are
currently left with no means to deprecate them from another x86_64
package. (RhBug:751574)
- While most scriptlets have both an interpreter and a body, neither
is strictly required: body can be omitted in cases like special
purpose executables (eg -p /sbin/ldconfig) and for interpreter,
/bin/sh is used if missing. This has been "broken" from somewhere
around rpm 4.7.x and nobody noticed :)
- This makes a huge difference in performance if you have lots
of unsigned packages (NOTFOUND verify result) or signed packages
without key (NOKEY verify result) installed, as we previously
kept checking the same headers over and over again.
- When new keys are added, any previous NOKEY results can become
invalid: either they become OK or FAIL, and its the FAIL case
we want to catch.
- For removed keys, previous OK could become NOKEY but that doesn't
make the header any less valid, so leave the cache alone on removal.
- Assume our home turf is safe enough for this - in order to reach
the rpmdb, headers must've gone through the more rigorous checking
that's done through the rpmReadPackageFile() paths, plus in
default configuration we'll be doing further verification on the
header before loading the headers so the risk seems acceptable
for the speed gain.
- regionSwab() calling dataLength() on headerImport() is one of the
busiest paths in rpm, and dataLength() on string types is a very
expensive call as it has to walk through the string looking for \0's.
The data size is actually available most of the time by just looking
at offsets (idea lifted from rpm5.org), which is an order of magnitude
faster than crawling string data. The downside (there always is one)
is that with offsets, string data is not validated to contain
sufficient number of \0's, which means malformed headers could cause
us to crash, burn and overflow when accessing the string data.
- The new "fast" mode enables offset-based calculation at callers
discretion, ie if the caller can reasonably assume the header is
sane (known to be previously validated etc), using the fast-flag
will make header loading/importing considerably faster.
For now, only headerImport() will use the fast mode but it might
make sense to remember the setting in the header and use for other
operations as well.
- Most callers need the size of the blob as well, which the unloader
internals know perfectly well but the interface doesn't support
passing it. So callers were forced to make a second call to
headerSizeof() to recalculate the size. Duh.
- Rename and export doHeaderUnload() as headerExport(), update internal
callers to use the new name. headerExport() is hopefully a bit
more obvious as a name than headerUnload() which doesn't actually
undo the effect of headerLoad() for that header, but merely exports
the data by serializing into on-disk format.
- Header size is not size_t really, its capped to fixed much lower
size. Use unsigned int to better match reality.
- Unlike headerLoad(), headerImport() takes a blob size argument
to allow sanity checking the size calculated from the blob itself
against the "physical" passed-in blob size so its a bit safer.
Note that header size is capped by various things - its not size_t.
- headerImport() also takes a flags argument to allow controlling
various aspects of importing.
- Implement "take copy of blob" as a flag to headerImport(), push
the copying into headerCreate() where we already know the blob
size, avoiding the need to do double-calculations on headerCopyLoad()..
- headerLoad() and headerCopyLoad() are now just compat wrappers
around the new interface.
- The pubkey headers have been rpm v3 all the way until now, whoops :)
Pull the actual key part of the header into immutable region and
stomp a sha1 digest on the result, allowing a (much) better
verification on loading. This part inspired by stumbling on a
related discussion on rpm5.org mailing list so credits where...
- Since we only insert either literally constant data or data retrieved
from the actual key into the immutable part of the header, the
calculated digest is constant for a given key regardless of where
and when it was imported. This gives some added verification and/or
cross-checking possibilities (eg was the imported key exactly the
same as what shipped etc)
- Create the header in makePubkeyHeader() as the name suggests,
return the newly created header to caller on success.
- Move the installtime & -tid addition to the "install" part,
makePubkeyHeader() only does the part that is specific to pubkey
headers, again as the name suggests.
- No functional changes
- Pubkey buildtime has until now been the time of import, which equals
install time/tid. Which is of course the time when that header
does get created, but it seems rather redundant to have the same
thing recorded in three places. Having the key creation time
easily (easier than un-hexifying the version string, duh)
available seems like a potentially useful thing. Buildtime is
"wrong" for this, but ... so is everything.
- With this change, the "meat" of the pubkey headers is now constant
and repeatable regardless of where and when a key gets imported,
so we could stomp a digest on it and it'd be unique for that
particular key everywhere.
- The userid has only been available in a mildly obfuscated format
through summary, but this seems like a useful thing to have in
a directly usable format without requiring callers to parse out
the gpg() wrapping around it.
- Yes its a wonky mapping, but so is everything else wrt
gpg-pubkeys, and adding a tag just for this also seems silly.
Using vendor tag could be another possibility, dunno.
- When calculating length of dribbles, we need to take into account the
size up to that point, otherwise the alignment can be wrong causing
the sizes not to add up.
- With that mystery solved, we can now make the final length check
as strict as it should be.
- Brainfart in previous commit (71c6b06b3f):
%pretrans failure should only cause that package to fail, not
abort the entire transaction. Doh.
- Failures are tracked via transaction elements but pre/posttrans
were specifically filtered out. All we need is removing that filtering
and the warn-only vs error logic in psm takes care of the rest.
The transaction.c changes in previous commit were just unnecessary.
- %pre and %preun scriptlets cause the package install/erase to fail,
whereas %pretrans return has simply been ignored ever since its
introduction somewhere in rpm <= 4.4.x. This is just inconsistent,
make %pretrans more like the other %pre-scriptlets. %posttrans
exit code is still essentially ignored, just like %post and %postun etc.
- This can obviously affect installability of existing packages: if
they have been careless about their %pretrans exit code or outright
relying on the "yes it spits errors in some situations but who cares"
behavior, they will now fail to install at all. The way to write
"portable" %pretrans scriptlets is ensuring non-error exit.
- Base64 is present in headers and all, it's only reasonable that
our API users have access to this functionality without having
to link to other libraries. Even if we didn't want to carry the
implementation forever in our codebase, we should provide a wrapping
for this (much like the other crypto stuff) for the reason stated above.
- A bigger issue is that our dirty little (badly hidden) secret was using
non-namespaced function names, clashing with at least beecrypt. And we
couldn't have made these internal-only symbols even on platforms that
support it, because they are used all over the place outside rpmio.
So... rename the b64 functions to rpmLikeNamingStyle and make 'em public.
No functional changes, just trivial renaming despite touching numerous
places.
- Change rpmVerifySignature() to take just the signature parameters
instead of the whole dig (this is an internal API so we're free
to mess with it) from which it only needed the signature params.
- The internal low-level verifySignature() is thus reduced to
to a call to rpmKeyringVerifySig() and spitting some silly
strings to msg.
- With this, keyring can now use and reuse the its internally stored
pgp key parameters instead of having to parse the same PGP packets
over and over. As a result, signature checking is faster now. Not
dramatically so but measurably nevertheless.
- Only call pgpDigGetParams() on the public key once we've at least
tried to fetch it via rpmKeyringLookup(). This way we dont assume
things about how pgpDig internal allocation is done - currently
it does return what's essentially a static pointer into pgpDig,
but this is not a reasonable assumption for an opaque type.
No functional changes.
- Hide allocation inside the helper, automatically free on failure
- Return pointer to the signature parameters on success to simplify
life for callers
- Don't bother checking or reporting the signature version: the
pgp parser errors out if it encounters unsupported version and
does not scrible anything to the version field in that case,
mumbling about "V0 signatures" is not particularly helpful.
- Log the bad package names from rpmpkgReadHeader() too
- Return a pointer to the signature part on success, hide allocation
(and free on failure) in the helper. Makes life a little bit
saner for the callers and limits the places where we access
the full pgpDig further.
- pgpVerifySig() is now just a dumb wrapper around pgpVerifySignature()
which does the real work.
- Update the sole caller to use the new interface instead, deprecate
the old dig interface.
- First steps towards getting rig of pgpDig which always was a
strange creature and now is nothing but a nuisance and obfuscation.
Yes keys and signatures walk hand in hand much of the time, but
they come from different sources and want to be handled as
separate data really.
- Eliminate bogus size calculations: we have a buffer of td->count size
that may or may not contain legal OpenPGP signature. Leave it up to
pgpPrtPkts() to validate & figure it out and check its return code instead,
eliminating need to repeat a bunch of tedious calculations here.
- Use non-zero signature version is used as a hint for valid signature,
should be "close enough" for the rest of the code.
readlink() never terminates the buffer.
Detected by "cppcheck" (git HEAD)
Signed-off-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
- String array size calculation could read one byte past data end
pointer when expected count and number of \0's disagree (ie invalid data)
due to while condition side-effects + bounds checking being in
the inner loop.
- Lift the string length calculation to inline helper function, used for
both string and string array types.
- Streamline the calculations:
- Eliminate unnecessary length increments, calculate the length
from pointer distance
- Eliminate end pointer NULL checking within the loop: when caller
doesn't supply end pointer, cap to HEADER_MAX_DATA (ie 16MB),
anything larger would trip up in later hdrchkData() checks anyway.
- Avoid the off-by-one by eliminating the problematic inner loop.
- Offset being within the data area doesn't help if the actual data doesn't
fit. Since the trailer size is well known, we can just as easily
make the check accurate to prevent reading beyond end of data in case
the offset is subtly wrong.
- In headerLoad(), region offset of zero doesn't need sanity checking,
only validate if its something else and do so accurately there too.
- Pretrans-dependencies are twisty little beasts unlike anything else...
When a pretrans-dependency provider is updated, the currently installed
version is the provider for that transaction, unlike others where
the packages from installing set act as providers for updates. So
when looking up pretrans deps, we must not prune the to-be-erased
packages from the db match iterators. As an added twist, we also
must not cache these non-pruned cases as it would mess up the
cache for "regular" dependencies.
- Fixes this case reported on fedora-devel:
http://lists.fedoraproject.org/pipermail/devel/2011-October/158058.html
- headerVerify() always returns with a message even for OK results,
which was masking the error message from headerLoad(), sometimes
giving not very helpful "headerRead failed: Header sanity check OK"
style messages.
- While we're on API killing spree... Exporting this was needless and
dumb to begin with (greetings to self in 2007...), bury it inside
depends.c as static and let rot there.
- Might be a better idea to kill it completely with some other
mechanism such as turning payload format into rpmlib() dependency
internally but just get it out of public sight for now.
Panu Matilainen