Skip to content

--network=pasta:--map-gw breaks subsequent network option parsing #22477

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Closed
kyrias opened this issue Apr 23, 2024 · 3 comments · Fixed by containers/common#1966
Closed

--network=pasta:--map-gw breaks subsequent network option parsing #22477

kyrias opened this issue Apr 23, 2024 · 3 comments · Fixed by containers/common#1966
Assignees
@Luap99
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. network Networking related issue or feature pasta pasta(1) bugs or features

Comments

@kyrias
Copy link

kyrias commented Apr 23, 2024

Issue Description

Using the --network=pasta:--map-gw option breaks the parsing of subsequent network options.

podman run -it --rm --network=pasta:--map-gw,-T,9129 archlinux /bin/bash

fails while this works:

podman run -it --rm --network=pasta:-T,9129,--map-gw archlinux /bin/bash

Steps to reproduce the issue

Steps to reproduce the issue

  1. podman run -it --rm --network=pasta:--map-gw,-T,9129 archlinux /bin/bash

Describe the results you received

Error: pasta failed with exit code 1:
Port forwarding mode 'none' conflicts with previous mode

Describe the results you expected

The container should start with the expected network configuration.

podman info output

host:
  arch: amd64
  buildahVersion: 1.35.3
  cgroupControllers:
  - cpuset
  - cpu
  - io
  - memory
  - hugetlb
  - pids
  - rdma
  - misc
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: /usr/bin/conmon is owned by conmon 1:2.1.10-1
    path: /usr/bin/conmon
    version: 'conmon version 2.1.10, commit: 2dcd736e46ded79a53339462bc251694b150f870'
  cpuUtilization:
    idlePercent: 88.05
    systemPercent: 3.9
    userPercent: 8.05
  cpus: 96
  databaseBackend: boltdb
  distribution:
    distribution: arch
    version: unknown
  eventLogger: journald
  freeLocks: 2048
  hostname: elokon-gitlab-runner2
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 6.8.7-arch1-1
  linkmode: dynamic
  logDriver: journald
  memFree: 106127396864
  memTotal: 269793722368
  networkBackend: netavark
  networkBackendInfo:
    backend: netavark
    dns:
      package: /usr/lib/podman/aardvark-dns is owned by aardvark-dns 1.10.0-2
      path: /usr/lib/podman/aardvark-dns
      version: aardvark-dns 1.10.0
    package: /usr/lib/podman/netavark is owned by netavark 1.10.3-1
    path: /usr/lib/podman/netavark
    version: netavark 1.10.3
  ociRuntime:
    name: crun
    package: /usr/bin/crun is owned by crun 1.14.4-1
    path: /usr/bin/crun
    version: |-
      crun version 1.14.4
      commit: a220ca661ce078f2c37b38c92e66cf66c012d9c1
      rundir: /run/user/0/crun
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL
  os: linux
  pasta:
    executable: /usr/bin/pasta
    package: /usr/bin/pasta is owned by passt 2024_04_05.954589b-1
    version: |
      pasta 2024_04_05.954589b
      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: /etc/containers/seccomp.json
    selinuxEnabled: false
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: /usr/bin/slirp4netns is owned by slirp4netns 1.2.3-1
    version: |-
      slirp4netns version 1.2.3
      commit: c22fde291bb35b354e6ca44d13be181c76a0a432
      libslirp: 4.7.0
      SLIRP_CONFIG_VERSION_MAX: 4
      libseccomp: 2.5.5
  swapFree: 134878064640
  swapTotal: 134896676864
  uptime: 1h 58m 13.00s (Approximately 0.04 days)
  variant: ""
plugins:
  authorization: null
  log:
  - k8s-file
  - none
  - passthrough
  - journald
  network:
  - bridge
  - macvlan
  - ipvlan
  volume:
  - local
registries: {}
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 0
    paused: 0
    running: 0
    stopped: 0
  graphDriverName: btrfs
  graphOptions: {}
  graphRoot: /var/lib/containers/storage
  graphRootAllocated: 1919342870528
  graphRootUsed: 996262256640
  graphStatus:
    Build Version: Btrfs v6.8
    Library Version: "102"
  imageCopyTmpDir: /var/tmp
  imageStore:
    number: 2
  runRoot: /run/containers/storage
  transientStore: false
  volumePath: /var/lib/containers/storage/volumes
version:
  APIVersion: 5.0.2
  Built: 1713438799
  BuiltTime: Thu Apr 18 11:13:19 2024
  GitCommit: 3304dd95b8978a8346b96b7d43134990609b3b29-dirty
  GoVersion: go1.22.2
  Os: linux
  OsArch: linux/amd64
  Version: 5.0.2

Podman in a container

No

Privileged Or Rootless

Rootless

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
@kyrias kyrias added the kind/bug Categorizes issue or PR as related to a bug. label Apr 23, 2024
@Luap99 Luap99 added network Networking related issue or feature pasta pasta(1) bugs or features labels Apr 24, 2024
@sbrivio-rh
Copy link
Collaborator

sbrivio-rh commented Apr 24, 2024 •
edited
Loading

Uh oh, I guess cmdArgs = append(cmdArgs[:i], cmdArgs[i+1:]...) in setupPasta() (in common/libnetwork/pasta/pasta.go) causes the next argument to be left unparsed -- we should use something safer to drop the --map-gw argument from the list once parsed, or perhaps two loops.

@Luap99
Copy link
Member

Luap99 commented Apr 24, 2024

@sbrivio-rh yes we shouldn't modify the slice while iterating over it. I work on a proper fix.

@Luap99 Luap99 self-assigned this Apr 24, 2024
Luap99 added a commit to Luap99/common that referenced this issue Apr 24, 2024
@Luap99
libnetwork/pasta: fix --map-gw parsing
c756850 
If a port option was given after --map-gw then parsing failed as the
next arg was always skipped due the modification of the slice.

Modifing the slice inside the loop is bad and does not do what some
might think. Append here basically creates a new slice (thus you always
have to assign the result to the variable) with the same pointer to the
same underlying array of data[1]. The loop however will still continue to
loop over the slice as it saw it at the begining of the loop.

So in the bug case the underlying array would look like this:
{"--config-net", "--map-gw", "-T", "80"}
and after the append call to remove --map-gw like this:
{"--config-net", "-T", "80", "80"}

The loop iterator has no idea this happen and just moves to the next
index 2 ("80") and thus we never passed "-T" causing this bug.

[1] https://go.dev/blog/slices-intro

Fixes containers/podman#22477

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

Luap99 commented Apr 24, 2024

Opened containers/common#1966

@Luap99 Luap99 mentioned this issue Apr 24, 2024
Merged
Luap99 added a commit to Luap99/common that referenced this issue Apr 24, 2024
@Luap99
libnetwork/pasta: fix --map-gw parsing
8e7d37f 
If a port option was given after --map-gw then parsing failed as the
next arg was always skipped due the modification of the slice.

Modifing the slice inside the loop is bad and does not do what some
might think. Append here basically creates a new slice (thus you always
have to assign the result to the variable) with the same pointer to the
same underlying array of data[1]. The loop however will still continue to
loop over the slice as it saw it at the begining of the loop.

So in the bug case the underlying array would look like this:
{"--config-net", "--map-gw", "-T", "80"}
and after the append call to remove --map-gw like this:
{"--config-net", "-T", "80", "80"}

The loop iterator has no idea this happen and just moves to the next
index 2 ("80") and thus we never passed "-T" causing this bug.

[1] https://go.dev/blog/slices-intro

Fixes containers/podman#22477

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99 Luap99 mentioned this issue May 21, 2024
Closed
@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Jul 24, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Jul 24, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Assignees

@Luap99 Luap99

Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. network Networking related issue or feature pasta pasta(1) bugs or features
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

libnetwork/pasta: fix --map-gw parsing Luap99/common
3 participants
@kyrias @Luap99 @sbrivio-rh