Skip to content

[v4.9-rhel] Fix: Remove appending rw as the default mount option#28197

Merged
Luap99 merged 1 commit intocontainers:v4.9-rhelfrom
TomSweeneyRedHat:dev/tsweeney/accel-596-v4.9-rhel
Mar 6, 2026
Merged

[v4.9-rhel] Fix: Remove appending rw as the default mount option#28197
Luap99 merged 1 commit intocontainers:v4.9-rhelfrom
TomSweeneyRedHat:dev/tsweeney/accel-596-v4.9-rhel

Conversation

@TomSweeneyRedHat
Copy link
Copy Markdown
Member

@TomSweeneyRedHat TomSweeneyRedHat commented Mar 4, 2026

The backstory for this is that runc 1.2 (opencontainers/runc#3967) fixed a long-standing bug in our mount flag handling (a bug that crun still has). Before runc 1.2, when dealing with locked mount flags that user namespaced containers cannot clear, trying to explicitly clearing locked flags (like rw clearing MS_RDONLY) would silently ignore the rw flag in most cases and would result in a read-only mount. This is obviously not what the user expects.

What runc 1.2 did is that it made it so that passing clearing flags like rw would always result in an attempt to clear the flag (which was not the case before), and would (in all cases) explicitly return an error if we try to clear locking flags. (This also let us finally fix a bunch of other long-standing issues with locked mount flags causing seemingly spurious errors).

The problem is that podman sets rw on all mounts by default (even if the user doesn't specify anything). This is actually a no-op in runc 1.1 and crun because of a bug in how clearing flags were handled (rw is the absence of MS_RDONLY but until runc 1.2 we didn't correctly track clearing flags like that, meaning that rw would literally be handled as if it were not set at all by users) but in runc 1.2 leads to unfortunate breakages and a subtle change in behaviour (before, a ro mount being bind-mounted into a container would also be ro -- though due to the above bug even setting rw explicitly would result in ro in most cases -- but with runc 1.2 the mount will always be rw even if the user didn't explicitly request it which most users would find surprising). By the way, this "always set rw" behaviour is a departure from Docker and it is not necesssary.

Fixes: https://issues.redhat.com/browse/RHEL-152630, https://issues.redhat.com/browse/RHEL-153022

(cherry picked from commit bf7dcd5)

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #00000 in commit message (if applicable)
  • Tests have been added/updated (or no tests are needed)
  • Documentation has been updated (or no documentation changes are needed)
  • All commits pass make validatepr (format/lint checks)
  • Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

None

@jankaluza
Copy link
Copy Markdown
Member

LGTM.

I wonder if we need to backport something more to fix the AWS build failure. I know it also happened on main branch and I've fixed it by rebasing to latest main in my PRs, so I think some commit in main fixed it?

@Luap99
Copy link
Copy Markdown
Member

Luap99 commented Mar 5, 2026

@timcoding1988 you need to backport your AWS changes to all active branches like I mentioned.

If I look at our cron job these are the branches we need for sure

Cron build 'v5.7' Failed: https://cirrus-ci.com/build/6672833683652608
Cron build 'v5.2-rhel' Failed: https://cirrus-ci.com/build/6212178442715136
Cron build 'v4.9-rhel' Failed: https://cirrus-ci.com/build/6170759254507520
Cron build 'v4.4.1-rhel' Failed: https://cirrus-ci.com/build/6575085999357952
Cron build 'v5.4-rhel' Failed: https://cirrus-ci.com/build/6032406026649600

Although I guess we don't need 5.7 but rather 5.8 now, I need to update the cron job setting

@timcoding1988
Copy link
Copy Markdown
Collaborator

@Luap99 sure ! all the backport pr for the active branch are out.

The backstory for this is that runc 1.2 (opencontainers/runc#3967)
fixed a long-standing bug in our mount flag handling (a bug that crun
still has). Before runc 1.2, when dealing with locked mount flags that
user namespaced containers cannot clear, trying to explicitly clearing
locked flags (like rw clearing MS_RDONLY) would silently ignore the rw
flag in most cases and would result in a read-only mount. This is
obviously not what the user expects.

What runc 1.2 did is that it made it so that passing clearing flags
like rw would always result in an attempt to clear the flag (which was
not the case before), and would (in all cases) explicitly return an
error if we try to clear locking flags. (This also let us finally fix a
bunch of other long-standing issues with locked mount flags causing
seemingly spurious errors).

The problem is that podman sets rw on all mounts by default (even if
the user doesn't specify anything). This is actually a no-op in
runc 1.1 and crun because of a bug in how clearing flags were handled
(rw is the absence of MS_RDONLY but until runc 1.2 we didn't correctly
track clearing flags like that, meaning that rw would literally be
handled as if it were not set at all by users) but in runc 1.2 leads to
unfortunate breakages and a subtle change in behaviour (before, a ro
mount being bind-mounted into a container would also be ro -- though
due to the above bug even setting rw explicitly would result in ro in
most cases -- but with runc 1.2 the mount will always be rw even if
the user didn't explicitly request it which most users would find
surprising). By the way, this "always set rw" behaviour is a departure
from Docker and it is not necesssary.

Fixes: https://issues.redhat.com/browse/RHEL-152630,
https://issues.redhat.com/browse/RHEL-153022

Signed-off-by: rcmadhankumar <madhankumar.chellamuthu@suse.com>
(cherry picked from commit bf7dcd5)
Signed-off-by: Tom Sweeney <tsweeney@redhat.com>
@TomSweeneyRedHat TomSweeneyRedHat force-pushed the dev/tsweeney/accel-596-v4.9-rhel branch from 8ab1a23 to f6b4d72 Compare March 5, 2026 21:39
@TomSweeneyRedHat
Copy link
Copy Markdown
Member Author

Rebased, hopefully that will pick up the AWS update

Copy link
Copy Markdown
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@Luap99 Luap99 merged commit 79517c7 into containers:v4.9-rhel Mar 6, 2026
6 checks passed
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.

5 participants