Age | Commit message (Collapse) | Author |
|
glibc 2.34 adds a closefrom() function call to close a range of file
descriptors. This one is problematic for us since pseudo can have its
own fds open in the close range.
To handle this we add a specific client side op, OP_CLOSEFROM, similar
to OP_CLOSE which closes the fds in the range which aren't pseudo fds.
This means manually closing some of the fds ourselves and then modifying
the call to closefrom for the rest. Not pretty but I'm struggling to see
a better way. It does mean msg/result is used in a new case to let the
caller know which fds to close as the range needs to change. This is
allowed after the previous static change.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
The address of msg is returned by the function in the OP_CHROOT case.
Whilst the way it is used means it is just a way of saying true/false,
the compiler doesn't know this and can warn and this isn't really
allowed.
The other pseudo funcitons in this area already use a static msg so
make this function match the others. There wouldn't be re-enterancy
in this context.
Need to be careful about clearing the data upon each function entry
and not just once at init.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
Fixes the following warning:
pseudo_client.c: In function ‘pseudo_root_path’:
pseudo_client.c:848:17: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
848 | pseudo_diag("couldn't allocate absolute path for '%s'.\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
849 | path);
| ~~~~~
Signed-off-by: Damian Wrobel <dwrobel@ertelnet.rybnik.pl>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
This change seems to cause faults in OE builds, not investigated why
but revert until we can debug. "bitbake linux-libc-headers" reproduces
all the time.
This reverts commit 04bca0f85d57da1d0ed0419780df296f8b0ff81d.
|
|
Fix some warnings reported by GCC 10.2.0:
* NULL pointer passed to '%s' format string parameter:
pseudo_client.c: In function ‘pseudo_root_path’:
pseudo_client.c:848:3: warning: ‘%s’ directive argument is null
[-Wformat-overflow=]
848 | pseudo_diag("couldn't allocate absolute path for '%s'.\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
849 | path);
* Return of variable with local storage duration:
pseudo_client.c: In function ‘pseudo_client_op’:
cc1: warning: function may return address of local variable
[-Wreturn-local-addr]
pseudo_client.c:1592:15: note: declared here
1592 | pseudo_msg_t msg = { .type = PSEUDO_MSG_OP };
Signed-off-by: Philip Lorenz <philip@bithub.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
The pseudo client path map stores paths that have been sanitised, but
in the ignored-path (PSEUDO_IGNORE_PATHS) codepath for open() calls this
sanitising wasn't performed so it is possible for paths that end with a
trailing slash to be entered.
This then subsequently interacts badly with path manipulation, resulting
in the situation where doing:
fd = open("/some/path/")
parent_fd = openat(fd, "../)
results in parent_fd actually pointing at /some/path still.
Solve this by ensuring that any trailing slashes are removed from the
path when adding to the map in the ignore short-circuit.
Also add a test case for this to ensure that it doesn't regress in the
future.
Signed-off-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
There is an issue in pseudo where if you open a file, rename the file,
then call fstat on the open fd, pseudo would thrown an abort. This is
because it needs to track the open fd mappings to files and it doesn't
update in the case of a rename.
Add code in pseudo to update the fd mappings in the case of a rename
call. Also add a test case.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
Pseudo will calculate the search paths used for passwd operations such
as lckpwdf and ulckpwdf using build_passwd_paths when it initiates. This
takes into account the chroot at the time.
The problem is that after a chroot is performed, pseudo continues to use
the search paths calculated from the start for lckpwdf and ulckpwdf.
This makes it write the pwd.lock files to a different sysroot if a chroot
is called during runtime.
This commit resolves that by calling build_passwd_paths again after
intercepting chroot calls so the search paths are up to date.
This bug manifests in Yocto when shadow is installed into an SDK target
rootfs. The postinst triggered will call shadow-native with -R to point
to the SDK target rootfs which in turn makes shadow call chroot to the
SDK target rootfs before it perform its actions including lckpwdf() and
ulckpwdf(). The lock files however will write instead to the normal image
target rootfs because it was specified in PSEUDO_PASSWD and was the first
path set when the pseudo environment was initiated. By rebuilding the
search path after chroot is applied, the lockfiles appear in the correct
rootfs.
Signed-off-by: Michael Ho <Michael.Ho@bmw.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
pseudo_get_value() returns newly allocated memory that the caller must free,
so add some free() calls.
Signed-off-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
Slightly alter a fallthrough comment so that GCC recognises it, and
add a default: case to a switch which explicitly only handles a few
values.
Signed-off-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
This also plugs a memory leak by making sure env is freed.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
The logfile path should be more visible so it's easier investigate the error.
Signed-off-by: Tomasz Dziendzielski <tomasz.dziendzielski@gmail.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
We're seeing systems in the wild (e.g. ubuntu 20.04) which call
with a dirfd set to the full filename and path set to "". Since
this seems to be expected to work, handle it accordingly.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
Consider what happens if a program does:
fd = fopen("A")
link("A", "B")
unlink("A")
fchown(fd)
Assuming we can't use the database, in order to handle this correctly,
we need to change the open fd to point at B when A us unlinked.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
Rather than mapping mismatched inode entries to paths, thrown an abort()
instead. Add a new result type to allow the server to pass back
this instruction to the client.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
Currently, pseudo considers any path accessed whist its running to be
a valid entry to track in its database. The way OpenEmbedded uses pseudo,
there are paths we care about accesses to from a pseudo perspective and paths
which we simply don't care about.
This patch adds a PSEUDO_IGNORE_PATHS environment variable which is a comma
separated list of path prefixes to ignore accesses to.
To do this, we add some functions which can check a path argument or a file
descriptor argument and use these in the pseudo wrappers where path or fd
arguments are present. Where paths are being ignored, we skip straight to
the underlying real function.
Psuedo needs to keep track of the open fd mappings to files so we still need
to allow those cases into the pseudo_op function. Specficially this means
OP_CLOSE, OP_OPEN, OP_DUP and OP_CHDIR.
Apart from OP_OPEN which could call the server, the other operations are client
side only so passed through. We 'tag' the functions using these operations so
that the path ignore code isn't triggered. For OP_OPEN we exit early and skip
the server op. We also have a catch all in client_op to ensure any operatings
we didn't manage to skip early still get skipped correctly.
OP_CHROOT is a special case. Where ignored path prefixes are used as a chroot,
for the lifetime of the chroot, the path is effectively dropped from the
PSEUDO_IGNORE_PATHS list. Whilst slightly counter intuaitive, this turned out
to be the most effective way to do things due to commands like useradd and
their use of chroots.
For sqlite3 and appropriate path filtering in OE, this took the database from
45,000 entries to about 180. For dbus this was 88,000 down to 760. Given the
number of client to server trips these numbers of paths involves, the win
is seemingly worthwhile.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
server shutdowns.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
This adds SPDX license headers to all source files in pseudo so license
identification models current best practise.
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
|
|
The path logic had gotten gradually more baroque, originally
from the need to handle trailing slashes in paths (because
we need "*/" to be a glob that matches only directories),
then from the need not to handle "regularfile/." and the like.
The revised solution is to move the invariant from "partial
path always ends with slash" to "appending always adds a
slash". Because this implies wanting to know the type of
the current partial path, and we also check the type of the
new path whenever we append an element, we now have a shared
stat buf for those. (With some manual tweaking, such as
an empty path is implicitly root and thus a directory, and
anything in which we want to expand a symlink is a
directory.)
Signed-off-by: Seebs <seebs@seebs.net>
|
|
FASTOP is logically fine as long as there's no errors, but
if there could be errors, we lose error detection. Responding
instantly with a trivial ACK for FASTOP messages slightly
reduces performance but improves reliability and seems to work
better.
Signed-off-by: Seebs <seebs@seebs.net>
|
|
If you're running pseudo in docker, a script that creates a pseudo
daemon can exit, causing docker to kill pseudo before it's done writing
the database.
Since the client sending the shutdown request doesn't have its socket
closed explicitly by the server, we can just read from the socket in
the client to create a delay until the actual exit, which can take
a while if there's an in-memory DB.
Signed-off-by: Seebs <seebs@seebs.net>
|
|
Fix provided by Patrick Ohly <patrick.ohly@intel.com>. This resolves
the actual cause of the path length mismatches, and explains why
I couldn't quite explain why the previous one had only sometimes
worked, also why it showed up on directories but not plain files.
Signed-off-by: Seebs <seebs@seebs.net>
|
|
So we had this really strange problem where, sometimes but not always,
pseudo would have strange problems on startup, where the pseudo server
would end up running under pseudo. And this produced the most fascinating
thing, which was:
unsetenv("LD_PRELOAD");
assert(getenv("LD_PRELOAD") == NULL);
for (int i = 0; environ[i]; ++i) {
assert(strncmp(environ[i], "LD_PRELOAD=", 11));
}
(pseudocode untested)
This would crash on the environ search. Because getenv() was not searching
environ.
WHAT.
So it turns out, *bash overrides getenv, setenv, and so on*. Under those
names. Hiding the glibc ones. And this creates horrible problems if you
assumed that your code could call those functions and expect them to work.
So as a workaround, pseudo now uses dlsym to find getenv, etc., from
glibc, and invokes those directly if possible. Also the client now uses
unwrapped fork/exec for spawning the server, which cleans up the
behavior of that code quite a bit.
|
|
Improved/simplified logic for the client spawning servers, to make it
(I hope) easier to see what it's trying to do and when. Also clearer
diagnostics about what may have gone wrong, and I don't check the pid file
unless there's a problem.
|
|
Server process now waits for its forked child when daemonizing, allowing
us to yield meaningful exit status. Lock is now taken by the child, since
it has a way to tell the parent about the exit status. (We send SIGUSR1 to
the server to cause the wait loop to stop when the client is ready to go.)
This allows us to switch to fcntl locking, which should in theory allow us
to run with the pseudo directory NFS-mounted. Woot!
Also mark a couple of overly spammy messages as PDBGF_VERBOSE to reduce the
volume of uninteresting dup spam when looking at client behaviors.
Client now uses execve to spawn server to work around a very strange behavior
of unsetenv.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
This is the big overhaul to have the server provide meaningful exit status
to clients.
In the process, I discovered that the server was running with signals blocked
if launched by a client, which is not a good thing, and prevented this from
working as intended.
Still looking to see why more than one server spawn seems to happen.
|
|
Improve event logging a little bit more, increase default event log size,
reduce retries (we shouldn't need that many if nothing's wrong), and make
the server log timestamps during database cleanup, since I'm suspicious of
that as a possible source of delays. Also cause server to emit a useful exit
status if it can't get a lock, and client to check server exit status when
spawning server.
|
|
For debugging the client/server startup, add an event logger to allow
better recording of events that we may, or may not, want to dump out
listings of later.
|
|
First, if aborting, display message even when no debugging is set, because
that's probably a big deal.
Second, if you use "pseudo <cmd>", try to die with the same signal that killed
the child process, if it died from a signal rather than exiting cleanly. (You
can't just pass the exit status out in that case, because exit(N) doesn't work
for N outside the range of non-signal exit statuses.)
|
|
There's a possible race condition if multiple clients try to start while
the server's down, especially if it's shutting down and thus holding a lock
but ignoring them. Logic altered to retry more often, at greater intervals.
Also, we are fine with being unable to spawn the server, because that can
happen if another client spawned it successfully. So we just retry sending
the message in a bit if we couldn't spawn a server, or immediately if we
could. (Because "could" spawn a server includes successfully communicating
with the newly-spawned server; the server-side code makes sure that the
child process won't exit before we expect such attempts to work, even if
they take a while.)
|
|
Race conditions exist when the server shutdown takes long enough for
three attempts to access the server to fail. Solution: Add a slight
delay to the retry. Delay is variable (using getpid()%5). (Not "random"
because I have no evidence that the process the client is running in
will have seeded RNG, and I don't want to seed it and possibly screw
them up).
|
|
In some cases, there can be a race with multiple clients trying to
start a server at once, and they should just retry their messages,
rather than aborting. I haven't been able to consistently reproduce
this, so it's not very well tested, but it seems reasonable.
|
|
Add some debug messages useful for tracking down xattr behaviors.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
When xattr emulation is used to store extended attributes, dummy
entries get made in the db using whatever UID/GID were in the real
stat buffer if no entry already existed. Change these to -1, and
treat -1 uid/gid as a missing entry for stat purposes.
xattrdb was not merging existing uid/gid values. Change this by
loading existing values to merge them in when executing chown/chmod
commands.
Newly-created files could end up with a filesystem mode of 0 if
you used umask, but this breaks xattrdb.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
Dropping the alloc from file paths meant that pseudo_exec_path
could end up just returning its original argument, which was
const-qualified, meaning its return should also be const-qualified.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
OP_OPEN and OP_EXEC are used only when logging. The server can now
tell the client (in response to initial ping) whether or not it is
logging, and if it isn't, the client doesn't send those messages.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
This is a moderately experimental feature which stores values in an
extended attribute called 'user.pseudo_data' instead of in the database.
Still missing: Database<->filesystem synchronization for this.
For at least some workloads, this can dramatically improve performance.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
Instead of allocating (and then freeing) these paths all the time,
use a rotating selection of buffers of fixed but probably large enough
size (the same size that would have been the maximum anyway in
general). With the exception of fts_open, there's no likely way to
end up needing more than two or three such paths at a time. fts_open
dups the paths since it could have a large number and need them for
a while. This dramatically reduces (in principle) the amount of allocation
and especially reallocation going on.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
A partially-implemented profiler for client time, which basically just
inserts (optional) gettimeofday calls in various places and stashes data
in a flat file containing one data block per pid.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
Having the same logic twice was sorta bugging me. Now the
function-like-macro is sorta bugging me, and I'll just let
it.
|
|
This is derived in significant part from contributions to oe-core
by Peter A. Bigot. I reworked the path routine a bit to use an
already duplicated string instead of allocating copies of parts of
it.
The first issue was just that there was a missing antimagic() around
some of the path operations. The second is that we wanted to have
a way to provide a fallback password file which isn't the host's,
but which can be used in the case where the target filesystem hasn't
got a password yet, for bootstrapping purposes. (So there's a minimal
password file that just has root, basically.)
Also, I noticed a design flaw, which is that if you ended up
calling pseudo_pwd_lck_open() twice in a row, the second time
through, pseudo would first check whether it had a path name
for the file (it does), and thus not allocate one, then call
the close routine (which frees it and nulls the pointer), then
open a new one... and not have a file name, so the next attempt
to close it wouldn't unlink the file. This shouldn't ever
come up in real code, but it was bugging me.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
|
|
We used to rely on filesystem operations to apply the umask when
appropriate, but when we started masking out 022, that stopped working.
Start watching umask.
|
|
Wait until the server has finished processing all of our messages
before exiting. Otherwise, it's possible for a command which sends
a no-response message and then exits to be followed by another
command which assumes the first one's done, and the second command's
messages can get processed first.
|
|
The xattr first-pass implementation was allocating a buffer to
hold the name and value for a set operation, then pseudo_client was
allocating *another* buffer to hold the path and those two values.
pseudo_client_op develops more nuanced argument handling, and also
uses a static buffer for the extended paths it sometimes needs. So
for the typical use case, only occasional operations will need to
reallocate/expand the buffer, and we'll be down to copying things
into that buffer once per operation, instead of having two alloc/free
pairs and two copies.
And of course, that wasn't two alloc/free pairs, it was one alloc/free
pair and one alloc without a free. Whoops.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
Issue #1: If an operation came in for an item with no path
provided by the wrapper, the client would not construct the
combined "path" value. Fixed, and missing paths are now
consistently handled as 0-byte paths.
Issue #2: The database code was assuming the values were
strings, and ignoring a specified length.
Issue #3: The computation of the length of the stored value
was off by one, because it was including the extra terminating
null the client added in case the value was a path.
With this in place, "cp -a" on CentOS is consistently
duplicating the system.posix_acl_access fields as expected,
but unfortunately not handling their permissions too.
(Intent is to translate a system.posix_acl_access setxattr
into corresponding permissions whenever possible.)
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
Initial, incomplete, support for extended attributes. Extended
attributes are implemented fairly naively, using a second table
in the file database using the primary file table's id as a
foreign key. The ON DELETE CASCADE behavior requires sqlite 3.6.19
or later with foreign key and trigger support compiled in.
To reduce round-trips, the client does not check for existing
attributes, but rather, sends three distinct set messages;
OP_SET_XATTR, OP_CREATE_XATTR, OP_REPLACE_XATTR. A SET message
always succeeds, a CREATE fails if the attribute already
exists, and a REPLACE fails if the attribute does not already
exist.
The /* flags */ feature of makewrappers is used to correct
path names appropriately, so all functions are already working
with complete paths, and can always use functions that work
on links; if they were supposed to dereference, the path
fixup code got that.
The xattr support is enabled, for now, conditional on
whether getfattr --help succeeds.
Not yet implemented: Translation for system.posix_acl_access,
which is used by "cp -a" (or "cp --preserve-all") on some
systems to try to copy modes.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
|
|
In some cases, we'd rather pseudo fail than fall back to using
/etc/passwd or /etc/group. Make the determination of what to fall
back to when neither PSEUDO_PASSWD nor a chroot directory contains
passwd/group files controllable by a configure-time flag, controlled
by --with-passwd-fallback= or --without-passwd-fallback.
|
|
|