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

storage-chown-by-maps doesnt handle -EOVERFLOW return by lgetxattr #1183

Closed
robertzaage opened this issue Mar 30, 2022 · 4 comments
Closed

Comments

@robertzaage
Copy link
Contributor

robertzaage commented Mar 30, 2022

This error occurs when running Podman under RHEL 8.5 (kernel-4.18.0-348.7.1.el8) in rootless mode usind an add. image store and having the keep-id option set. The add. image store was created following this guide by another rootless user. Error handling by storage-chown-by-maps seems to be a problem at this point.

If the the owner of the fsuser namespace can't be apped into the current user namespace -EOVERFLOW is returned by lgetxattr. In the past (before kernel patch was applied), already handled -EOPNOTSUPP was returned. In current configuration it won't handle the return and dies with following error message:

$: podman run --userns keep-id ubi8 id
Error: error creating container storage: exit status 1: error during chown: storage-chown-by-maps: lgetxattr usr/bin/ping: value too large for defined data type

Trace:

[pid 1533307] lgetxattr("/user/container/store/overlay/1eef0109c76f08116d55ca3ac2e091ec1a44ebf0572f82958936375f990dfe23/diff/usr/bin/ping", "security.capability", NULL, 0) = -1 EOVERFLOW (Value too large for defined data type)
[pid 1533358] <... newfstatat resumed>{st_mode=S_IFLNK|0777, st_size=4, ...}, AT_SYMLINK_NOFOLLOW) = 0
[pid 1533307] writev(11, [{iov_base="\20\0\0\0\265\377\377\377\330\361\1\0\0\0\0\0", iov_len=16}], 1) = 16
[pid 1533355] <... lgetxattr resumed>, 0xc00024d080, 128) = -1 EOVERFLOW (Value too large for defined data type)

Bug was introduced by this kernel patch:
torvalds/linux@f2b00be

RHEL 8.4 / kernel-4.18.0-305.el8
https://access.redhat.com/labs/rhcb/RHEL-8.4/kernel-4.18.0-305.el8/source/blob/security/commoncap.c#L434

if (!rootid_owns_currentns(kroot)) {
    kfree(tmpbuf);
    return -EOPNOTSUPP;
}

RHEL 8.5 / kernel-4.18.0-348.7.1.el8
https://access.redhat.com/labs/rhcb/RHEL-8.5/kernel-4.18.0-348.7.1.el8/source/blob/security/commoncap.c#L446

if (!rootid_owns_currentns(kroot)) {
    size = -EOVERFLOW;
    goto out_free;
}

Affected Code Part:
https://github.com/containers/storage/blob/main/drivers/chown_unix.go#L87

Possible Fix:
https://github.com/robertzaage/storage/pull/1/files#diff-e60559509109696ada9e7f412d847e6a7eac43123aca579745842747a55471fb

Additional Informations:

host:
  arch: amd64
  buildahVersion: 1.22.3
  cgroupControllers: []
  cgroupManager: cgroupfs
  cgroupVersion: v1
  conmon:
    package: conmon-2.0.29-1.module+el8.5.0+12582+56d94c81.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.0.29, commit: 0f5bee61b18d4581668e5bf18b910cda3cff5081'
  cpus: 48
  distribution:
    distribution: '"rhel"'
    version: "8.5"
  eventLogger: file
  hostname: ########
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 531072
      size: 65536
    uidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 531072
      size: 65536
  kernel: 4.18.0-348.7.1.el8_5.x86_64
  linkmode: dynamic
  memFree: 194385903616
  memTotal: 201133006848
  ociRuntime:
    name: runc
    package: runc-1.0.2-1.module+el8.5.0+12582+56d94c81.x86_64
    path: /usr/bin/runc
    version: |-
      runc version 1.0.2
      spec: 1.0.2-dev
      go: go1.16.7
      libseccomp: 2.5.1
  os: linux
  remoteSocket:
    path: /run/user/1000/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_NET_RAW,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: true
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: false
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.1.8-1.module+el8.5.0+12582+56d94c81.x86_64
    version: |-
      slirp4netns version 1.1.8
      commit: d361001f495417b880f20329121e3aa431a8f90f
      libslirp: 4.4.0
      SLIRP_CONFIG_VERSION_MAX: 3
      libseccomp: 2.5.1
  swapFree: 33536602112
  swapTotal: 33536602112
  uptime: 34h 21m 45.13s (Approximately 1.42 days)
registries:
  search:
  - registry.fedoraproject.org
  - registry.access.redhat.com
  - registry.centos.org
  - docker.io
store:
  configFile: /home/user/.config/containers/storage.conf
  containerStore:
    number: 1
    paused: 0
    running: 0
    stopped: 1
  graphDriverName: overlay
  graphOptions:
    overlay.imagestore: /opt/container/store
    overlay.mount_program:
      Executable: /usr/bin/fuse-overlayfs
      Package: fuse-overlayfs-1.7.1-1.module+el8.5.0+12582+56d94c81.x86_64
      Version: |-
        fusermount3 version: 3.2.1
        fuse-overlayfs: version 1.7.1
        FUSE library version 3.2.1
        using FUSE kernel interface version 7.26
  graphRoot: /home/user/.local/share/containers/storage
  graphStatus:
    Backing Filesystem: xfs
    Native Overlay Diff: "false"
    Supports d_type: "true"
    Using metacopy: "false"
  imageStore:
    number: 2
  runRoot: /run/user/1000/containers
  volumePath: /home/user/.local/share/containers/storage/volumes
version:
  APIVersion: 3.3.1
  Built: 1632213702
  BuiltTime: Tue Sep 21 10:41:42 2021
  GitCommit: ""
  GoVersion: go1.16.7
  OsArch: linux/amd64
  Version: 3.3.1
@rhatdan
Copy link
Member

rhatdan commented Mar 30, 2022

Is this a bug in the kernel or a bug in storage?

@robertzaage
Copy link
Contributor Author

robertzaage commented Mar 30, 2022

Is this a bug in the kernel or a bug in storage?

IMO this one is storage related

@giuseppe
Copy link
Member

giuseppe commented Apr 6, 2022

the fix seems correct to me. @robertzaage could you please open a PR for containers/storage since you already have a patch?

@giuseppe
Copy link
Member

giuseppe commented Apr 6, 2022

another reference to this issue: https://bugzilla.redhat.com/show_bug.cgi?id=2072452

robertzaage added a commit to robertzaage/storage that referenced this issue Apr 6, 2022
robertzaage added a commit to robertzaage/storage that referenced this issue Apr 6, 2022
robertzaage added a commit to robertzaage/storage that referenced this issue Apr 6, 2022
robertzaage added a commit to robertzaage/storage that referenced this issue Apr 6, 2022
Closes containers#1183]

Signed-off-by: robertzaage <38880514+robertzaage@users.noreply.github.com>
robertzaage added a commit to robertzaage/storage that referenced this issue Apr 6, 2022
Closes containers#1183]

Signed-off-by: robertzaage <38880514+robertzaage@users.noreply.github.com>
robertzaage added a commit to robertzaage/storage that referenced this issue Apr 6, 2022
giuseppe added a commit that referenced this issue Apr 7, 2022
fix storage-chown-by-maps doesnt handle -EOVERFLOW on lgetxattr [Closes #1183]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants