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

Don't let plugins fail due to read-only fs #3239

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dmnks
Copy link
Contributor

@dmnks dmnks commented Aug 12, 2024

Read-only file systems fall into the same category as unsupported ones (i.e. writing the desired file metadata to them is meaningless) so handle those gracefully in the plugin hooks, too.

This is useful e.g. when preparing a system for IMA by reinstalling the desired packages with the IMA plugin enabled, including ones that own paths such as /mnt which are meant to be used as mount points, often read-only (e.g. when mounting an ISO).

Fixes: RHEL-33726

Read-only file systems fall into the same category as unsupported ones
(i.e. writing the desired file metadata to them is meaningless) so
handle those gracefully in the plugin hooks, too.

This is useful e.g. when preparing a system for IMA by reinstalling the
desired packages with the IMA plugin enabled, including ones that own
paths such as /mnt which are meant to be used as mount points, often
read-only (e.g. when mounting an ISO).

Fixes: RHEL-33726
@dmnks
Copy link
Contributor Author

dmnks commented Aug 12, 2024

One scenario just came to mind:

  1. You mount a directory (as read-only) over another directory that's owned by an installed package
  2. The plugin skips the directory silently
  3. You unmount the directory
  4. The original directory does not have the file attributes applied (!)

In a way, this is a security risk, too.

Maybe we should make this into a warning after all...

@pmatilai
Copy link
Member

These should probably be handled the same as fsm does (see commit 09d554d): if there's an error applying, skip if the attributes match, otherwise it's an error.

@dmnks
Copy link
Contributor Author

dmnks commented Aug 13, 2024

Ack, I'll look into that.

@dmnks dmnks added the DONT DO NOT merge, for whatever reason label Aug 13, 2024
@pmatilai
Copy link
Member

Of course, the issue of underlying directory not getting updated goes for any (temporary) mounts we happen to touch during a transaction. This may call for stepping back a bit for an overall strategy (fancy expression for "I don't know" 😆 )

We know what mounts we cross paths with, and we technically even know which ones are readonly mounts. We just don't make a good use of that information: if we try to install something on such a filesystem we fail out with "not enough space" which is a bit confusing. Maybe we could automatically add readonly mounts to %_netsharedpath so they get skipped, but we shouldn't do that for an fs where we're actually trying to install to, only for the mountpoints.

And, that still leaves the question of what to do with any temporary mounts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DONT DO NOT merge, for whatever reason
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants