Skip to content

Commit 458fcaa

Browse files
giuseppeclaude
andcommitted
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 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
1 parent a003f56 commit 458fcaa

File tree

3 files changed

+68
-7
lines changed

3 files changed

+68
-7
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ require (
3333
github.com/gorilla/schema v1.4.1
3434
github.com/hashicorp/go-multierror v1.1.1
3535
github.com/hugelgupf/p9 v0.3.1-0.20250420164440-abc96d20b308
36+
github.com/jinzhu/copier v0.4.0
3637
github.com/json-iterator/go v1.1.12
3738
github.com/kevinburke/ssh_config v1.5.0
3839
github.com/klauspost/pgzip v1.2.6
@@ -130,7 +131,6 @@ require (
130131
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
131132
github.com/hashicorp/go-retryablehttp v0.7.8 // indirect
132133
github.com/inconshreveable/mousetrap v1.1.0 // indirect
133-
github.com/jinzhu/copier v0.4.0 // indirect
134134
github.com/klauspost/compress v1.18.4 // indirect
135135
github.com/kr/fs v0.1.0 // indirect
136136
github.com/lufia/plan9stats v0.0.0-20240909124753-873cd0166683 // indirect

pkg/specgen/generate/container_create.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/containers/podman/v6/pkg/specgen"
1919
"github.com/containers/podman/v6/pkg/specgenutil"
2020
"github.com/containers/podman/v6/pkg/util"
21+
"github.com/jinzhu/copier"
2122
"github.com/opencontainers/runtime-spec/specs-go"
2223
"github.com/opencontainers/selinux/go-selinux"
2324
"github.com/sirupsen/logrus"
@@ -740,12 +741,7 @@ func Inherit(infra *libpod.Container, s *specgen.SpecGenerator, rt *libpod.Runti
740741
compatibleOptions.SelinuxOpts = append(compatibleOptions.SelinuxOpts, s.SelinuxOpts...)
741742
compatibleOptions.Volumes = append(compatibleOptions.Volumes, s.Volumes...)
742743

743-
compatByte, err := json.Marshal(compatibleOptions)
744-
if err != nil {
745-
return nil, nil, nil, err
746-
}
747-
err = json.Unmarshal(compatByte, s)
748-
if err != nil {
744+
if err := applyInfraInherit(compatibleOptions, s); err != nil {
749745
return nil, nil, nil, err
750746
}
751747

@@ -760,3 +756,8 @@ func Inherit(infra *libpod.Container, s *specgen.SpecGenerator, rt *libpod.Runti
760756
}
761757
return options, infraSpec, compatibleOptions, nil
762758
}
759+
760+
// applyInfraInherit copies the InfraInherit fields into the SpecGenerator.
761+
func applyInfraInherit(compatibleOptions *libpod.InfraInherit, s *specgen.SpecGenerator) error {
762+
return copier.CopyWithOption(s, compatibleOptions, copier.Option{IgnoreEmpty: true})
763+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//go:build !remote && (linux || freebsd)
2+
3+
package generate
4+
5+
import (
6+
"testing"
7+
8+
"github.com/containers/podman/v6/libpod"
9+
"github.com/containers/podman/v6/pkg/specgen"
10+
spec "github.com/opencontainers/runtime-spec/specs-go"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// TestApplyInfraInheritMountOptionsDoNotLeak verifies that mount options from
16+
// one mount do not leak into another when calling applyInfraInherit.
17+
func TestApplyInfraInheritMountOptionsDoNotLeak(t *testing.T) {
18+
compatibleOptions := &libpod.InfraInherit{
19+
Mounts: []spec.Mount{
20+
{Destination: "/mylog", Source: "/a", Type: "bind"},
21+
{Destination: "/mytmp", Source: "/b", Type: "bind", Options: []string{"ro"}},
22+
},
23+
}
24+
25+
s := &specgen.SpecGenerator{}
26+
s.Mounts = []spec.Mount{
27+
{Destination: "/mytmp", Source: "/b", Type: "bind", Options: []string{"ro"}},
28+
{Destination: "/mylog", Source: "/a", Type: "bind"},
29+
}
30+
31+
err := applyInfraInherit(compatibleOptions, s)
32+
require.NoError(t, err)
33+
34+
for _, m := range s.Mounts {
35+
if m.Destination == "/mylog" {
36+
assert.Empty(t, m.Options,
37+
"/mylog should have no options; ro from /mytmp leaked")
38+
}
39+
if m.Destination == "/mytmp" {
40+
assert.Equal(t, []string{"ro"}, m.Options,
41+
"/mytmp should keep its ro option")
42+
}
43+
}
44+
}
45+
46+
// TestApplyInfraInheritDoesNotOverwriteSeccomp verifies that applyInfraInherit
47+
// does not overwrite a pre-set SeccompProfilePath when the infra container has
48+
// no seccomp profile (empty string).
49+
func TestApplyInfraInheritDoesNotOverwriteSeccomp(t *testing.T) {
50+
compatibleOptions := &libpod.InfraInherit{}
51+
52+
s := &specgen.SpecGenerator{}
53+
s.SeccompProfilePath = "localhost/seccomp.json"
54+
55+
err := applyInfraInherit(compatibleOptions, s)
56+
require.NoError(t, err)
57+
58+
assert.Equal(t, "localhost/seccomp.json", s.SeccompProfilePath,
59+
"SeccompProfilePath should not be overwritten by empty infra value")
60+
}

0 commit comments

Comments
 (0)