From 458fcaa1ba7a010b0d542b453da3d687e59845fb Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 9 Mar 2026 23:08:17 +0000 Subject: [PATCH] specgen: fix pod mount options leaking between mounts Replace the JSON marshal/unmarshal round-trip in Inherit() with copier.Copy. json.Unmarshal reuses existing slice backing arrays and does not zero struct fields absent from the JSON (omitempty), so mount options like "ro" from one mount would leak into another mount at the same backing-array position. Fixes the case where running: podman run --pod mypod \ --mount type=bind,src=/a,target=/mylog \ --mount type=bind,src=/b,target=/mytmp,ro=true \ alpine touch /mylog/a incorrectly fails with "Read-only file system" because /mylog inherits "ro" from /mytmp. Fixes: https://issues.redhat.com/browse/RHEL-154348 Co-Authored-By: Claude Opus 4.6 Signed-off-by: Giuseppe Scrivano --- go.mod | 2 +- pkg/specgen/generate/container_create.go | 13 ++-- pkg/specgen/generate/container_create_test.go | 60 +++++++++++++++++++ 3 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 pkg/specgen/generate/container_create_test.go diff --git a/go.mod b/go.mod index c7ad979cd1..360a975363 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/gorilla/schema v1.4.1 github.com/hashicorp/go-multierror v1.1.1 github.com/hugelgupf/p9 v0.3.1-0.20250420164440-abc96d20b308 + github.com/jinzhu/copier v0.4.0 github.com/json-iterator/go v1.1.12 github.com/kevinburke/ssh_config v1.5.0 github.com/klauspost/pgzip v1.2.6 @@ -130,7 +131,6 @@ require ( github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-retryablehttp v0.7.8 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/jinzhu/copier v0.4.0 // indirect github.com/klauspost/compress v1.18.4 // indirect github.com/kr/fs v0.1.0 // indirect github.com/lufia/plan9stats v0.0.0-20240909124753-873cd0166683 // indirect diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 3f1e59495c..3848164889 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -18,6 +18,7 @@ import ( "github.com/containers/podman/v6/pkg/specgen" "github.com/containers/podman/v6/pkg/specgenutil" "github.com/containers/podman/v6/pkg/util" + "github.com/jinzhu/copier" "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux" "github.com/sirupsen/logrus" @@ -740,12 +741,7 @@ func Inherit(infra *libpod.Container, s *specgen.SpecGenerator, rt *libpod.Runti compatibleOptions.SelinuxOpts = append(compatibleOptions.SelinuxOpts, s.SelinuxOpts...) compatibleOptions.Volumes = append(compatibleOptions.Volumes, s.Volumes...) - compatByte, err := json.Marshal(compatibleOptions) - if err != nil { - return nil, nil, nil, err - } - err = json.Unmarshal(compatByte, s) - if err != nil { + if err := applyInfraInherit(compatibleOptions, s); err != nil { return nil, nil, nil, err } @@ -760,3 +756,8 @@ func Inherit(infra *libpod.Container, s *specgen.SpecGenerator, rt *libpod.Runti } return options, infraSpec, compatibleOptions, nil } + +// applyInfraInherit copies the InfraInherit fields into the SpecGenerator. +func applyInfraInherit(compatibleOptions *libpod.InfraInherit, s *specgen.SpecGenerator) error { + return copier.CopyWithOption(s, compatibleOptions, copier.Option{IgnoreEmpty: true}) +} diff --git a/pkg/specgen/generate/container_create_test.go b/pkg/specgen/generate/container_create_test.go new file mode 100644 index 0000000000..134e1a415c --- /dev/null +++ b/pkg/specgen/generate/container_create_test.go @@ -0,0 +1,60 @@ +//go:build !remote && (linux || freebsd) + +package generate + +import ( + "testing" + + "github.com/containers/podman/v6/libpod" + "github.com/containers/podman/v6/pkg/specgen" + spec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestApplyInfraInheritMountOptionsDoNotLeak verifies that mount options from +// one mount do not leak into another when calling applyInfraInherit. +func TestApplyInfraInheritMountOptionsDoNotLeak(t *testing.T) { + compatibleOptions := &libpod.InfraInherit{ + Mounts: []spec.Mount{ + {Destination: "/mylog", Source: "/a", Type: "bind"}, + {Destination: "/mytmp", Source: "/b", Type: "bind", Options: []string{"ro"}}, + }, + } + + s := &specgen.SpecGenerator{} + s.Mounts = []spec.Mount{ + {Destination: "/mytmp", Source: "/b", Type: "bind", Options: []string{"ro"}}, + {Destination: "/mylog", Source: "/a", Type: "bind"}, + } + + err := applyInfraInherit(compatibleOptions, s) + require.NoError(t, err) + + for _, m := range s.Mounts { + if m.Destination == "/mylog" { + assert.Empty(t, m.Options, + "/mylog should have no options; ro from /mytmp leaked") + } + if m.Destination == "/mytmp" { + assert.Equal(t, []string{"ro"}, m.Options, + "/mytmp should keep its ro option") + } + } +} + +// TestApplyInfraInheritDoesNotOverwriteSeccomp verifies that applyInfraInherit +// does not overwrite a pre-set SeccompProfilePath when the infra container has +// no seccomp profile (empty string). +func TestApplyInfraInheritDoesNotOverwriteSeccomp(t *testing.T) { + compatibleOptions := &libpod.InfraInherit{} + + s := &specgen.SpecGenerator{} + s.SeccompProfilePath = "localhost/seccomp.json" + + err := applyInfraInherit(compatibleOptions, s) + require.NoError(t, err) + + assert.Equal(t, "localhost/seccomp.json", s.SeccompProfilePath, + "SeccompProfilePath should not be overwritten by empty infra value") +}