cannot assign device path with colon

[martinetd]

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

There is no way to escape colons in paths provided in podman run --device ...

Steps to reproduce the issue:

1. Find some device with colons or make one. /dev/*/by-path are good candidates:

   $ ls -l /dev/*/*by*/*:*
   lrwxrwxrwx 1 root root  9 Sep 16 16:08 /dev/disk/by-id/usb-Generic-_Micro_SD_M2_058F84688461-0:1 -> ../../sdb
   lrwxrwxrwx 1 root root  9 Sep 16 16:08 /dev/disk/by-id/usb-Generic-_SD_MMC_058F84688461-0:0 -> ../../sda
   lrwxrwxrwx 1 root root  9 Sep 16 16:08 /dev/disk/by-path/pci-0000:00:14.0-usb-0:2.1:1.0-scsi-0:0:0:0 -> ../../sda
   lrwxrwxrwx 1 root root  9 Sep 16 16:08 /dev/disk/by-path/pci-0000:00:14.0-usb-0:2.1:1.0-scsi-0:0:0:1 -> ../../sdb
   lrwxrwxrwx 1 root root 13 Jul 27 11:00 /dev/disk/by-path/pci-0000:3d:00.0-nvme-1 -> ../../nvme0n1
   ...

2. try to pass it to a container with podman run --device=/dev/... image

Describe the results you received:

stat xxx: no such file or directory with path truncated before colon. Nothing I could think of as escape worked.

Describe the results you expected:

There should be a way of not splitting at colons, e.g. by allowing escapes or encoding : differently (à la systemd-escape, e.g. : -> \x3a ?)

Additional information you deem important (e.g. issue happens only occasionally):

Workaround: do lookup manually and create with mknod or link destination, and use another fixed path within container Solution: iiuc the fix would be in ParseDevice in pkg/specgen/generate/config_linux.go, replacing strings.Split(device, ":") by something slightly smarter. I didn't try.

Output of podman version:

tried on 3.2.3 but the code is still the same as of today's master 8e2d25e937

[vrothberg]

Thanks for reaching out, @martinetd.

--volume has the same limitation which can be overcome by using --mount. @giuseppe, should we add a "device" type for --mount?

[Luap99]

Does this work with docker?

[martinetd]

@Luap99 good point, I didn't think of trying. I also cannot seem to get it to work with docker either. Searching for docker instead of podman yields a few unsatisfying results: https://stackoverflow.com/a/56746298 (workaround: make a symlink without dash next to the real one) https://github.com/moby/moby/issues/39293 (no workaround given, but hints at adding a device type for --mount)

I'd be happy with the mount option if given, I have the same problem described in the question as my video devices don't have a fixed /dev/videoX number but the path is not usable. I can workaround by using a different path inside the container and doing the readlink myself (or additional link like described in the SO answer), but being able to use the original paths would be great.

[giuseppe]

@giuseppe, should we add a "device" type for --mount?

That seems useful to me.

For now, a possible workaround could be something like:

podman run --device /proc/self/fd/3:/dev/path .... 3</dev/path

[github-actions[bot]]

A friendly reminder that this issue had no activity for 30 days.

[rhatdan]

Did https://github.com/containers/podman/pull/11912 fix this issue?

[martinetd]

I didn't test, but I assume it wouldn't as device is split directly with strings.Split(device, ":") in containers/common/pkg/config/config.go if I understand this correctly. Given that there is also the path inside the container to consider (host path, container path, permission) I guess a similar fix would also work as long as the path within the container does not contain colons, but frankly that seems optimistic -- I think having have escaping (e.g. \\ -> \, \: -> :) if we go this way makes more sense. OTOH, the --mount type device that was suggested earlier is probably just as simple (hm, I didn't think about it, but in the case of mount then , becomes a forbidden character?)

[github-actions[bot]]

A friendly reminder that this issue had no activity for 30 days.

[rhatdan]

--mount is the only answer for now.

[martinetd]

Hi,

ignoring the fact that there is no --mount type=device type (I don't see the difference with type=bind), -- mount has the same problem with commas that --device has with colon:

# touch /tmp/test,comma
# podman run -ti  --rm --mount type=bind,src=/tmp/test,comma,dst=/tmp/test docker.io/alpine
Error: comma: invalid mount option

I think @giuseppe 's suggestion of using /proc/self/fd/x is more robust, even if it's a bit of a pain to use and might have side-effects for some devices (some drivers don't like having a device open multiple times and podman does not close the fd after getting infos from that path)

That being said, I honestly don't have a better idea; the only safe delimiter would be a NUL byte and that doesn't work well as a command-line interface, so allowing to escape colons (\: -> :, \\ -> \) would probably be ideal. Since you wrote for now, would you be open to a patch? The amount of hits I get grepping for Split.*: is scary, but if it's ok to just add this substitution for --device (in containers/common/pkg/config/config.go) and --volume (in I assume cmd/podman/common/create_opts.go ?) I can probably make some time for that early next year.

[giuseppe]

I think @giuseppe 's suggestion of using /proc/self/fd/x is more robust, even if it's a bit of a pain to use and might have side-effects for some devices (some drivers don't like having a device open multiple times and podman does not close the fd after getting infos from that path)

another alternative could be a symlink. You need to clean it up afterwards but at least it doesn't cause side effects like opening the device earlier

[ciandonovan]

I think @giuseppe 's suggestion of using /proc/self/fd/x is more robust, even if it's a bit of a pain to use and might have side-effects for some devices (some drivers don't like having a device open multiple times and podman does not close the fd after getting infos from that path)

another alternative could be a symlink. You need to clean it up afterwards but at least it doesn't cause side effects like opening the device earlier

Using a symlink doesn't work unfortunately.

I have a device /dev/iio:device0 from an Intel Realsense 435i symlinked by udev to /dev/realsense_accel_3d.

I pass the device using the argument --device=/dev/realsense_accel_3d:/dev/device0:rw and get the error Error: invalid device specification: /dev/iio:device0:/dev/device0:rw.

Clearly Podman resolves the symlink first before mounting the device, and so the issue with the colon in the device name persists.

[martinetd]

hmm, that definitely used to work, but it looks like it's been broken for a while alright... it does work with volume so I guess it's not all lost.

For devices now I just gave up and our current recommendation for our users is to just pass /dev as a volume and trust --device-cgroup-rule -- it's not as practical as passing a single device, but it works in practice. Also, if you just really need a single device you can extract major/minor and recreate another node with the same name instead of ln, but that's a bit heavyweight...

[ciandonovan]

Currently we use udev rules to create well-known named symlinks for devices so a subset can easily be passed through to separate containers, so using --device-cgroup-rule with a major number won't work that well unfortunately, and the minor numbers are too unpredictable.

Will experiment with renaming the node itself with udev or recreating a new node under a different name as you suggested, thanks!

[rhatdan]

We could change the code to see if a device on the host exists with the name with colons before splitting it.

Check if --device /dev/iio:device0:/dev/device0:rw

Could check if a device named /dev/iio:device0:/dev/device0:rw /dev/iio:device0:/dev/device0 /dev/iio:device0 /dev/iio On the host and then interpret the remainder of the option.

[ciandonovan]

We could change the code to see if a device on the host exists with the name with colons before splitting it.

Check if --device /dev/iio:device0:/dev/device0:rw

Could check if a device named /dev/iio:device0:/dev/device0:rw /dev/iio:device0:/dev/device0 /dev/iio:device0 /dev/iio On the host and then interpret the remainder of the option.

I was worried there might be an issue if both the below devices existed

$ ls -l /dev
iio
iio:device

and I pass to Podman --device /dev/iio:device:rw, (wanting /dev/iio on the host to be mapped to device in the working directory in the container) that there might be some ambiguity, but thankfully it doesn't look like Podman supports relative paths on the container side for devices, so not an issue.

Anyway, thanks Dan, what you suggested looks like a solid approach.

[Luap99]

I don't think we should stat the devices ever. This stuff never works via remote client and as @ciandonovan said this is ambiguous.

I would recommend to use a csv like parser, like we do for --mount (https://github.com/containers/podman/pull/13969). With that we could support something like --device '"/dev/path:with:colon":"/dev/contianer:here":rw'

Also the man pages are very clear about --device resolving symlinks:

Note: if host-device is a symbolic link then it is resolved first. The container only stores the major and minor numbers of the host device.

This seems be the case for a long time (https://github.com/containers/podman/issues/4550)

[rhatdan]

Well I believe we currently stat the device if all of the information is not provided, like major/minor number.

Bottom line is this is something we should support.

Using CSV seems like a better alternative. Now we just need someone to implement it. My point is that this is a bug and something that we should fix.

[cyqsimon]

Bump. Ran into this today with such a USB device. Fortunately for me I got away with using /dev/by-uuid/, but it won't work for everyone.

May I suggest taking inspiration from sed? Usually people would write something like this: s/foo/bar/, but you're actually allowed to use anything as a delimiting character, for example: s|/path/to/foo|/path/to/bar|.

So what I'm proposing is an alternative grammar like this: <D><HOST_PATH>[<D><GUEST_PATH>][<D><PERMS>]<D>, where <D> is the delimiting character. Note that the string starts and ends with <D>, which can be used as the switching condition during parsing. For example, OP could then have something like this:

--device "|/dev/disk/by-id/usb-Generic-_SD_MMC_058F84688461-0:0|rw|"

It's also probably best to only support a small subset of symbols for the user to choose from for <D> instead of any character to minimise the likelihood of accidental breakages (e.g. we don't want --device read-only-device:r to be misunderstood as "HOST_PATH is ead-only-device:").

[rhatdan]

Nice idea, It might be nice to support : | ;

[Luap99]

I would not invent a new syntax that is not used anywhere in podman. As mentioned above we can use a csv parser like we did for mount. Also I see no reason why we coudn't add --mount type=device,...

[rhatdan]

That works as well, and is a better solution.

[cyqsimon]

I've given it some thought and have come to agree that a CSV parser is indeed a better solution, mostly because it extends the old syntax rather than inventing something entirely new.

Also out of curiosity, what's Podman's policy regarding implementing Docker-divergent features like this? Is there some sort of coordination with Docker people?

[rhatdan]

The Podman team attempts to match Docker functionality when they make changes like buildkit supported features. But there is no effort to go the other way. When Podman adds features they do not necessarily show up back in Docker.