Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support alternate passwd/group locations in tmpfiles #16512

Merged
merged 2 commits into from Jul 18, 2020

Conversation

keszybz
Copy link
Member

@keszybz keszybz commented Jul 18, 2020

No description provided.

The test binary has two modes: in the default argument-less mode, it
just checks that "root" can be resolved. When invoked manually, a root
prefix and user/group names can be specified.
This changes the code to allow looking at multiple files with different
prefixes, but uses "/etc" and "/usr/lib". rpm-ostree uses
/usr/lib/{passwd,group} with nss-altfiles. I see no harm in simply trying both
paths on all systems.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1857530.

A minor memory leak is fixed: hashmap_put() returns -EEXIST is the key is
present *and* and the value is different. It return 0 if the value is the
same. Thus, we would leak the user/group name if it was specified multiple
times with the same uid/gid. I opted to remove the warning message completely:
with multiple files it is reasonable to have the same name defined more than
once. But even with one file the warning is dubious: all tools that read those
files deal correctly with duplicate entries and we are not writing a linter.
@yuwata
Copy link
Member

yuwata commented Jul 18, 2020

LGTM.

@yuwata yuwata merged commit 4573592 into systemd:master Jul 18, 2020
@keszybz keszybz deleted the offline-passwd-altfiles branch July 21, 2020 09:06
uid_t *ret_uid,
Hashmap **cache) {
static int open_passwd_file(const char *root, const char *fname, FILE **ret_file) {
const char *p = prefix_roota(root, fname);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is weird. both the left hand and the right hand side allocate from the stack... this smells like something the compile might choke on, the same way as it chokes on alloca() in parameters of function call expressions.

i'd really split this up in two lines to avoid any confusion

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, prefix_roota() can't fail, no need to catch errors here...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prepped #16534 which fixes both issues indirectly and is a lot more correct in other ways, too.

poettering added a commit to poettering/systemd that referenced this pull request Jul 21, 2020
In case the passwd/group file is symlinked, follow things correctly.

Follow-up for: systemd#16512
Addresses: systemd#16512 (comment)
poettering added a commit that referenced this pull request Jul 21, 2020
In case the passwd/group file is symlinked, follow things correctly.

Follow-up for: #16512
Addresses: #16512 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants