From 83863a6863e5f506cf7181da2e8226696043bcd2 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 28 Jun 2024 16:14:28 +0200 Subject: [PATCH 1/2] specgen: parse devices even with privileged set When a users asks for specific devices we should still add them and not ignore them just because privileged adds all of them. Most notably if you set --device /dev/null:/dev/test you expect /dev/test in the container, however as we ignored them this was not the case. Another side effect is that the input was not validated at at all. This leads to confusion as descriped in the issue. Fixes #23132 Signed-off-by: Paul Holzinger --- pkg/specgen/generate/oci_linux.go | 31 ++++++++++++++----------------- pkg/util/utils_linux.go | 1 - test/e2e/run_test.go | 13 +++++++++++++ 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/pkg/specgen/generate/oci_linux.go b/pkg/specgen/generate/oci_linux.go index da9b30ec6b..d6247bbf67 100644 --- a/pkg/specgen/generate/oci_linux.go +++ b/pkg/specgen/generate/oci_linux.go @@ -254,24 +254,21 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt } var userDevices []spec.LinuxDevice - - if !s.IsPrivileged() { - // add default devices from containers.conf - for _, device := range rtc.Containers.Devices.Get() { - if err = DevicesFromPath(&g, device); err != nil { - return nil, err - } + // add default devices from containers.conf + for _, device := range rtc.Containers.Devices.Get() { + if err = DevicesFromPath(&g, device); err != nil { + return nil, err } - if len(compatibleOptions.HostDeviceList) > 0 && len(s.Devices) == 0 { - userDevices = compatibleOptions.HostDeviceList - } else { - userDevices = s.Devices - } - // add default devices specified by caller - for _, device := range userDevices { - if err = DevicesFromPath(&g, device.Path); err != nil { - return nil, err - } + } + if len(compatibleOptions.HostDeviceList) > 0 && len(s.Devices) == 0 { + userDevices = compatibleOptions.HostDeviceList + } else { + userDevices = s.Devices + } + // add default devices specified by caller + for _, device := range userDevices { + if err = DevicesFromPath(&g, device.Path); err != nil { + return nil, err } } s.HostDeviceList = userDevices diff --git a/pkg/util/utils_linux.go b/pkg/util/utils_linux.go index c36c424d4e..85d819b9b2 100644 --- a/pkg/util/utils_linux.go +++ b/pkg/util/utils_linux.go @@ -106,7 +106,6 @@ func AddPrivilegedDevices(g *generate.Generator, systemdMode bool) error { if err != nil { return err } - g.ClearLinuxDevices() if rootless.IsRootless() { mounts := make(map[string]interface{}) diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 6fd7fb1334..95a2ab61e1 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -1687,6 +1687,19 @@ VOLUME %s`, ALPINE, volPath, volPath) Expect(session).Should(ExitCleanly()) }) + It("podman run --device and --privileged", func() { + session := podmanTest.Podman([]string{"run", "--device", "/dev/null:/dev/testdevice", "--privileged", ALPINE, "ls", "/dev"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + Expect(session.OutputToString()).To(ContainSubstring(" testdevice "), "our custom device") + // assumes that /dev/mem always exists + Expect(session.OutputToString()).To(ContainSubstring(" mem "), "privileged device") + + session = podmanTest.Podman([]string{"run", "--device", "invalid-device", "--privileged", ALPINE, "ls", "/dev"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(125, "stat invalid-device: no such file or directory")) + }) + It("podman run --replace", func() { // Make sure we error out with --name. session := podmanTest.Podman([]string{"create", "--replace", ALPINE, "/bin/sh"}) From 9814ed40c730aa31d5f3b30bd80f200f79ef116d Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 28 Jun 2024 16:20:24 +0200 Subject: [PATCH 2/2] docs: --network remove missing leading sentence This senetence does not add any value and instead confuses users as it suggest that the name is somhow special and related to bridge networks which is not the case. Using either the name or id is fine as described in the sentence before. Signed-off-by: Paul Holzinger --- docs/source/markdown/options/network.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/markdown/options/network.md b/docs/source/markdown/options/network.md index 8a63e2d323..2e67cc0df6 100644 --- a/docs/source/markdown/options/network.md +++ b/docs/source/markdown/options/network.md @@ -17,7 +17,7 @@ Valid _mode_ values are: For example, to set a static ipv4 address and a static mac address, use `--network bridge:ip=10.88.0.10,mac=44:33:22:11:00:99`. -- _\_**[:OPTIONS,...]**: Connect to a user-defined network; this is the network name or ID from a network created by **[podman network create](podman-network-create.1.md)**. Using the network name implies the bridge network mode. It is possible to specify the same options described under the bridge mode above. Use the **--network** option multiple times to specify additional networks. \ +- _\_**[:OPTIONS,...]**: Connect to a user-defined network; this is the network name or ID from a network created by **[podman network create](podman-network-create.1.md)**. It is possible to specify the same options described under the bridge mode above. Use the **--network** option multiple times to specify additional networks. \ For backwards compatibility it is also possible to specify comma-separated networks on the first **--network** argument, however this prevents you from using the options described under the bridge section above. - **none**: Create a network namespace for the container but do not configure network interfaces for it, thus the container has no network connectivity.