Skip to content

Consume the defaults mount option instead of passing it as data - #5502

Open
wangyusheng1985 wants to merge 1 commit into
opencontainers:mainfrom
wangyusheng1985:contribution-5500
Open

wangyusheng1985 wants to merge 1 commit into
opencontainers:mainfrom
wangyusheng1985:contribution-5500

Conversation

@wangyusheng1985

Copy link
Copy Markdown

defaults is a recognized mount option, but its flag value is zero. The parser only consumed options with a nonzero flag, so defaults was appended to the filesystem data string and forwarded to tmpfs.

Recognized options are now consumed even when the flag is zero. Unknown options are still passed through as data, and nonzero flags keep their set and clear behavior.

Fixes #5500

defaults is a recognized mount option with a zero flag. The parser treated
a zero flag as unknown and appended it to filesystem data, so tmpfs mounts
received defaults as a data option. Consume recognized zero-flag options
and keep unknown options in the data string.

Fixes opencontainers#5500

Signed-off-by: 玉升 <wyshdiy@163.com>
@kolyshkin

Copy link
Copy Markdown
Contributor

@wangyusheng1985, please fix your Signed-off-by: to be the same as the Author:

Comment on lines -1144 to +1158
// If the option does not exist in the mountFlags table,
// or the flag is not supported on the platform,
// then it is a data value for a specific fs type.
if f, exists := mountFlags[o]; exists && f.flag != 0 {
// Options absent from the mountFlags table are data for a specific fs type.
// A recognized option is consumed even when its flag is zero: "defaults"
// is a no-op and must not be forwarded as filesystem data (for example to tmpfs).
if f, exists := mountFlags[o]; exists {
// FIXME: The *atime flags are special (they are more of an enum
// with quite hairy semantics) and thus arguably setting some of
// them should clear unrelated flags.
if f.clear {
m.Flags &= ^f.flag
m.ClearedFlags |= f.flag
} else {
m.Flags |= f.flag
m.ClearedFlags &= ^f.flag
if f.flag != 0 {
if f.clear {
m.Flags &= ^f.flag
m.ClearedFlags |= f.flag
} else {
m.Flags |= f.flag
m.ClearedFlags &= ^f.flag
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is too much of a fix. What's sufficient is:

@@ -1141,10 +1141,9 @@ func parseMountOptions(options []string) *configs.Mount {
      initMaps()
      for _, o := range options {
-             // If the option does not exist in the mountFlags table,
-             // or the flag is not supported on the platform,
-             // then it is a data value for a specific fs type.
-             if f, exists := mountFlags[o]; exists && f.flag != 0 {
+             // If the option does not exist in the mountFlags table,
+             // then it is a data value for a specific fs type.
+             if f, exists := mountFlags[o]; exists {
                      // FIXME: The *atime flags are special (they are more of an enum
                      // with quite hairy semantics) and thus arguably setting some of
                      // them should clear unrelated flags.

and maybe add defaults to the table, for clarity:

  "defaults":      {false, 0}, // no-op, must not be passed as data

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(The above also removes the obsoleted "flags is not supported on the platform" as modern x/sys/unix have all the flags defined).

if strings.Contains(m.Data, "defaults") {
t.Errorf("defaults must not be passed as mount data, got %q", m.Data)
}
if !strings.Contains(m.Data, "size=1m") || !strings.Contains(m.Data, "mode=777") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would rather check for a specific string here:

if m.Data != "size=1m,mode=777" {

@kolyshkin kolyshkin added the backport/1.5-todo A PR in main branch which needs to be backported to release-1.5 label Sep 30, 2026
@kolyshkin

Copy link
Copy Markdown
Contributor

@wangyusheng1985 PTAL ^^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/1.5-todo A PR in main branch which needs to be backported to release-1.5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

runc passes the recognized defaults mount option to tmpfs as filesystem data

2 participants