Skip to content

Commit

Permalink
Fix (relocatable) packages that own / in fsm
Browse files Browse the repository at this point in the history
During payload processing, if we encounter a file entry corresponding to
the prefix directory itself, instead of referring to it relative to its
parent directory's file descriptor, we pass the absolute / path to the
*at() calls which then makes them fall back to path-based operation.

This isn't a huge problem as long as we're installing into the real root
prefix, however if the package is built as relocatable and the prefix is
overridden, we still operate on the real "/" instead of the new prefix.

This is wrong and even causes an installation failure if the relocatable
package is installed as a regular user who doesn't have write access for
the real root.

Fix this by simply passing "." instead of "/" to these *at() calls, to
always ensure fd-based operation.

However, this alone isn't sufficient since we can't delete the relocated
prefix directory itself this way when removing the package.  This is due
to the fact that unlinkat(2) with AT_REMOVEDIR actually does a rmdir(2),
and that fails with EINVAL if called with the "." path.

Therefore, when removing a package and encountering the prefix directory
itself, stop the traversal in ensureDir() one level higher than usual so
that we refer to the prefix directory relative to its parent directory
via normal fd-based operation.

Lastly, don't even attempt to remove the prefix if it's *not* relocated.
This is safer and also gets rid of a spurious warning that's printed
when removing a non-relocatable package owning / as the root user.

Such packages (that include the sole / in their %files section) perhaps
aren't exactly common and those that exist aren't normally installed or
uninstalled on a production system (such as the "filesystem" package on
Fedora), however it's apparently used as a shorthand for including all
files in a relocatable package, as evidenced by TBD.

Either way, this is a regression introduced by the fsm rework addressing
the symlink CVEs (see rpm-software-management#1919 for details).

Fixes: TBD
  • Loading branch information
dmnks committed Jul 16, 2024
1 parent c38abe2 commit b51166a
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 8 deletions.
46 changes: 38 additions & 8 deletions lib/fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ static int fsmClose(int *wfdp);
static char * fsmFsPath(rpmfi fi, const char * suffix)
{
const char *bn = rpmfiBN(fi);
return rstrscat(NULL, *bn ? bn : "/", suffix ? suffix : "", NULL);
return rstrscat(NULL, *bn ? bn : ".", suffix ? suffix : "", NULL);
}

static int fsmLink(int odirfd, const char *opath, int dirfd, const char *path)
Expand Down Expand Up @@ -373,7 +373,7 @@ static int fsmDoMkDir(rpmPlugins plugins, int dirfd, const char *dn,
}

static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create,
int quiet, int *dirfdp)
int quiet, int *dirfdp, char **dirfnp)
{
char *sp = NULL, *bn;
char *apath = NULL;
Expand All @@ -390,6 +390,12 @@ static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create,
char *dp = path;

while ((bn = strtok_r(dp, "/", &sp)) != NULL) {
/* stop at parent dir if requested (and return its name) */
if (dirfnp && !(*sp)) {
*dirfnp = xstrdup(bn);
break;
}

fd = fsmOpenat(dirfd, bn, oflags, 1);
/* assemble absolute path for plugins benefit, sigh */
apath = rstrscat(&apath, "/", bn, NULL);
Expand Down Expand Up @@ -417,6 +423,8 @@ static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create,
if (rc) {
fsmClose(&fd);
fsmClose(&dirfd);
if (dirfnp)
free(*dirfnp);
} else {
rc = 0;
}
Expand Down Expand Up @@ -945,7 +953,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
int mayopen = 0;
int fd = -1;
rc = ensureDir(plugins, rpmfiDN(fi), 0,
(fp->action == FA_CREATE), 0, &di.dirfd);
(fp->action == FA_CREATE), 0, &di.dirfd, NULL);

/* Directories replacing something need early backup */
if (!rc && !fp->suffix && fp != firstlink) {
Expand Down Expand Up @@ -1061,7 +1069,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,

if (!fp->skip) {
if (!rc)
rc = ensureDir(NULL, rpmfiDN(fi), 0, 0, 0, &di.dirfd);
rc = ensureDir(NULL, rpmfiDN(fi), 0, 0, 0, &di.dirfd, NULL);

/* Backup file if needed. Directories are handled earlier */
if (!rc && fp->suffix)
Expand Down Expand Up @@ -1089,7 +1097,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
struct filedata_s *fp = &fdata[fx];

/* If the directory doesn't exist there's nothing to clean up */
if (ensureDir(NULL, rpmfiDN(fi), 0, 0, 1, &di.dirfd))
if (ensureDir(NULL, rpmfiDN(fi), 0, 0, 1, &di.dirfd, NULL))
continue;

if (fp->stage > FILE_NONE && !fp->skip) {
Expand Down Expand Up @@ -1124,6 +1132,8 @@ int rpmPackageFilesRemove(rpmts ts, rpmte te, rpmfiles files,
int fx = -1;
struct filedata_s *fdata = (struct filedata_s *)xcalloc(fc, sizeof(*fdata));
int rc = 0;
int rootdir = 0;
char *dirfn = NULL;

while (!rc && (fx = rpmfiNext(fi)) >= 0) {
struct filedata_s *fp = &fdata[fx];
Expand All @@ -1133,8 +1143,24 @@ int rpmPackageFilesRemove(rpmts ts, rpmte te, rpmfiles files,
continue;

fp->fpath = fsmFsPath(fi, NULL);
rootdir = rstreq(fp->fpath, ".");

/*
* Handle the case where the path is the root directory itself. If
* we're in the real root, don't even attempt to unlink it. Otherwise,
* we're in a relocated root which we do want to unlink, so manually
* signal a directory change for "di" to be re-populated with the
* parent directory (trying to unlink the "." path would result in an
* EINVAL error).
*/
if (rstreq(rpmfiDN(fi), "/"))
continue;
else if (rootdir)
onChdir(fi, &di.dirfd);

/* If the directory doesn't exist there's nothing to clean up */
if (ensureDir(NULL, rpmfiDN(fi), 0, 0, 1, &di.dirfd))
if (ensureDir(NULL, rpmfiDN(fi), 0, 0, 1, &di.dirfd,
(rootdir ? &dirfn : NULL)))
continue;

rc = fsmStat(di.dirfd, fp->fpath, 1, &fp->sb);
Expand All @@ -1145,13 +1171,14 @@ int rpmPackageFilesRemove(rpmts ts, rpmte te, rpmfiles files,
rc = rpmpluginsCallFsmFilePre(plugins, fi, fp->fpath,
fp->sb.st_mode, fp->action);

rc = fsmBackup(di.dirfd, fi, fp->action);
if (!rootdir)
rc = fsmBackup(di.dirfd, fi, fp->action);

/* Remove erased files. */
if (fp->action == FA_ERASE) {
int missingok = (rpmfiFFlags(fi) & (RPMFILE_MISSINGOK | RPMFILE_GHOST));

rc = fsmRemove(di.dirfd, fp->fpath, fp->sb.st_mode);
rc = fsmRemove(di.dirfd, (rootdir ? dirfn : fp->fpath), fp->sb.st_mode);

/*
* Missing %ghost or %missingok entries are not errors.
Expand Down Expand Up @@ -1197,6 +1224,9 @@ int rpmPackageFilesRemove(rpmts ts, rpmte te, rpmfiles files,
rpm_loff_t amount = rpmfiFC(fi) - rpmfiFX(fi);
rpmpsmNotify(psm, RPMCALLBACK_UNINST_PROGRESS, amount);
}

free(dirfn);
dirfn = NULL;
}

for (int i = 0; i < fc; i++)
Expand Down
17 changes: 17 additions & 0 deletions tests/data/SPECS/reloc2.spec
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Name: reloc2
Version: 1.0
Release: 1
Summary: Testing relocation behavior with prefix ownership
License: GPL
Prefix: /
BuildArch: noarch

%description
%{summary}.

%install
mkdir -p $RPM_BUILD_ROOT/foo
touch $RPM_BUILD_ROOT/foo/bar

%files
%{prefix}
46 changes: 46 additions & 0 deletions tests/rpmi.at
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,52 @@ runroot rpm -U --relocate /opt/bin=/bin \
[])
RPMTEST_CLEANUP

AT_SETUP([rpm -i relocatable package owning prefix])
AT_KEYWORDS([install relocate])
RPMDB_INIT

runroot rpmbuild --quiet -bb /data/SPECS/reloc2.spec

RPMTEST_CHECK([
runroot rpm -U /build/RPMS/noarch/reloc2-1.0-1.noarch.rpm
],
[0],
[],
[])

RPMTEST_CHECK([
runroot rpm -e reloc2
],
[0],
[],
[])
RPMTEST_CLEANUP

AT_SETUP([rpm -i relocatable package owning prefix (user)])
AT_KEYWORDS([install relocate])
RPMTEST_USER
RPMDB_INIT

runroot rpmbuild --quiet -bb /data/SPECS/reloc2.spec

RPMTEST_CHECK([
runroot_user \
rpm -U --prefix \$PWD/root --dbpath \$PWD/rpmdb \
/build/RPMS/noarch/reloc2-1.0-1.noarch.rpm
],
[0],
[],
[])

RPMTEST_CHECK([
runroot_user rpm -e --dbpath \$PWD/rpmdb reloc2
runroot_user test ! -d \$PWD/root
],
[0],
[],
[])
RPMTEST_CLEANUP

AT_SETUP([rpm -i with/without --excludedocs])
AT_KEYWORDS([install excludedocs])
RPMTEST_CHECK([
Expand Down

0 comments on commit b51166a

Please sign in to comment.