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

libct/cg/sd/dbus: fix NewDbusConnManager #3006

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Jun 7, 2021

Noticed (in cri-o/cri-o@5f97b2f#r646855529)
that the check of trying to use both rootful and rootless
in NewDbusConnManager never worked, as we never set dbusInited to true. 🤦🏻

Do that. While at it, protect this with the mutex (against the
case of two goroutines simultaneously calling NewDbusConnManager).
This is a rare call, so taking read-only then read-write mutex does not
make sense.

Fixes: c7f847e

(Practically, this is not an issue as I don't expect this (someone using rootless and rootful at the same time) to happen -- but if the check is there it should at least work)

@kolyshkin kolyshkin added this to the 1.0.0 milestone Jun 7, 2021
@kolyshkin kolyshkin requested review from AkihiroSuda and cyphar June 7, 2021 19:21
Noticed that the check of trying to use both rootful and rootless
in NewDbusConnManager never worked, as we never set dbusInited to true.

Do that. While at it, protect this with the mutex (against the
case of two goroutines simultaneously calling NewDbusConnManager).
This is a rare call, so taking read-only then read-write mutex does not
make sense.

Fixes: c7f847e

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

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

LGTM.

@cyphar cyphar closed this in dcdf6b6 Jun 9, 2021
@cyphar cyphar merged commit dcdf6b6 into opencontainers:master Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants