Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runroot is limited to 50 characters #22272

Closed
rstreif opened this issue Apr 5, 2024 · 10 comments · Fixed by #22277
Closed

runroot is limited to 50 characters #22272

rstreif opened this issue Apr 5, 2024 · 10 comments · Fixed by #22277
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@rstreif
Copy link

rstreif commented Apr 5, 2024

Issue Description

The --runroot argument only accepts paths that are less than 50 characters. Albeit, --root and --tmpdir do not seem to have that limitation.

Steps to reproduce the issue

Run podman with --runroot with a path that is longer than 50 characters.

Describe the results you received

Error: the specified runroot is longer than 50 characters

Describe the results you expected

runroot should work with paths longer than 50 characters.

podman info output

host:
  arch: amd64
  buildahVersion: 1.33.7
  cgroupControllers:
  - cpu
  - io
  - memory
  - pids
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: conmon-2.1.10-1.fc38.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.1.10, commit: '
  cpuUtilization:
    idlePercent: 96.96
    systemPercent: 1.92
    userPercent: 1.12
  cpus: 128
  databaseBackend: sqlite
  distribution:
    distribution: fedora
    variant: workstation
    version: "38"
  eventLogger: journald
  freeLocks: 2048
  hostname: threaddy
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 6.7.9-100.fc38.x86_64
  linkmode: dynamic
  logDriver: journald
  memFree: 15228346368
  memTotal: 134918381568
  networkBackend: netavark
  networkBackendInfo:
    backend: netavark
    dns:
      package: aardvark-dns-1.10.0-1.fc38.x86_64
      path: /usr/libexec/podman/aardvark-dns
      version: aardvark-dns 1.10.0
    package: netavark-1.10.3-1.fc38.x86_64
    path: /usr/libexec/podman/netavark
    version: netavark 1.10.3
  ociRuntime:
    name: crun
    package: crun-1.14.4-1.fc38.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 1.14.4
      commit: a220ca661ce078f2c37b38c92e66cf66c012d9c1
      rundir: /run/user/1000/crun
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +LIBKRUN +WASM:wasmedge +YAJL
  os: linux
  pasta:
    executable: /usr/bin/pasta
    package: passt-0^20240220.g1e6f92b-1.fc38.x86_64
    version: |
      pasta 0^20240220.g1e6f92b-1.fc38.x86_64
      Copyright Red Hat
      GNU General Public License, version 2 or later
        <https://www.gnu.org/licenses/old-licenses/gpl-2.0.html>
      This is free software: you are free to change and redistribute it.
      There is NO WARRANTY, to the extent permitted by law.
  remoteSocket:
    exists: false
    path: /run/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: false
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: true
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.2.2-1.fc38.x86_64
    version: |-
      slirp4netns version 1.2.2
      commit: 0ee2d87523e906518d34a6b423271e4826f71faf
      libslirp: 4.7.0
      SLIRP_CONFIG_VERSION_MAX: 4
      libseccomp: 2.5.3
  swapFree: 12854718464
  swapTotal: 12884893696
  uptime: 263h 13m 43.00s (Approximately 10.96 days)
  variant: ""
plugins:
  authorization: null
  log:
  - k8s-file
  - none
  - passthrough
  - journald
  network:
  - bridge
  - macvlan
  - ipvlan
  volume:
  - local
registries:
  search:
  - registry.fedoraproject.org
  - registry.access.redhat.com
  - docker.io
  - quay.io
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 0
    paused: 0
    running: 0
    stopped: 0
  graphDriverName: vfs
  graphOptions: {}
  graphRoot: /tmp/containers/root
  graphRootAllocated: 67459190784
  graphRootUsed: 57450496
  graphStatus: {}
  imageCopyTmpDir: /var/tmp
  imageStore:
    number: 4
  runRoot: /tmp/containers/run
  transientStore: true
  volumePath: /tmp/containers/root/volumes
version:
  APIVersion: 4.9.4
  Built: 1711446116
  BuiltTime: Tue Mar 26 02:41:56 2024
  GitCommit: ""
  GoVersion: go1.21.8
  Os: linux
  OsArch: linux/amd64
  Version: 4.9.4

Podman in a container

No

Privileged Or Rootless

None

Upstream Latest Release

Yes

Additional environment details

Additional environment details

Additional information

Additional information like issue happens only occasionally or issue happens with a particular architecture or on a particular setting

@rstreif rstreif added the kind/bug Categorizes issue or PR as related to a bug. label Apr 5, 2024
@rhatdan
Copy link
Member

rhatdan commented Apr 5, 2024

@mheon PTAL

@rhatdan
Copy link
Member

rhatdan commented Apr 5, 2024

Although I think you might get into trouble with socket paths on long lengths.

@mheon
Copy link
Member

mheon commented Apr 5, 2024 via email

@Luap99
Copy link
Member

Luap99 commented Apr 5, 2024

I think Dan is correct that this limitation is in place because of Unix socket path length restrictions. Unfortunately not something we can easily work around, kernel level restriction on how long we can make those paths.

We already have workarounds in place for this restriction. If there are still places where this is needed we can convert them to use the workaround (open the path under /proc/self/fd).

Also 50 is just such a arbitrary value. I would say let's remove it and if something fails we can fix this on case by cases bases IMO.

@Luap99
Copy link
Member

Luap99 commented Apr 5, 2024

Original change seems to be #1704, and mentions issue with conmon attach socket paths. I would hope those are fixed by now.

@Luap99
Copy link
Member

Luap99 commented Apr 5, 2024

Removing the check and using a base path > 108 to be greater than the socket path length seems to work fine to run a container so I am going to open a PR and remove this limitation.

Luap99 added a commit to Luap99/libpod that referenced this issue Apr 5, 2024
This was added ages ago in commit c65b359, however in the meantime
both podman and conmon can support longer socket paths as they use a
workaround to open the path via /proc/self/fd, see openUnixSocket() in
libpod/oci_conmon_attach_linux.go

Thus this restriction is not needed anymore and we can drop a workaround
in the tests.

Fixes containers#22272

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Luap99 added a commit to Luap99/libpod that referenced this issue Apr 5, 2024
This was added ages ago in commit c65b359, however in the meantime
both podman and conmon can support longer socket paths as they use a
workaround to open the path via /proc/self/fd, see openUnixSocket() in
libpod/oci_conmon_attach_linux.go

Thus this restriction is not needed anymore and we can drop a workaround
in the tests.

Fixes containers#22272

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@rstreif
Copy link
Author

rstreif commented Apr 5, 2024

Thank you for looking into this. I was not aware that this was due to a UNIX socket path restriction. However, that restriction as far as I know is defined in un.h as #define UNIX_PATH_MAX 108. Ideally, podman would use that value, as @Luap99 seems to indicate.

@Luap99
Copy link
Member

Luap99 commented Apr 5, 2024

Well we must reserve space for the actual path to the socket, because we use container ids and they are 64 chars in hex so we exceed the 108 anyway even with the 50 chars limit. But regardless this kernel limitation is stupid and we have worked around that since #8933 apparently.

So the solution is to just drop the limit.

@rhatdan
Copy link
Member

rhatdan commented Apr 5, 2024

SGTM

@rstreif
Copy link
Author

rstreif commented Apr 9, 2024

@Luap99, thank you for resolving this. It works in my environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants