From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 5BF145A004E for ; Thu, 08 Aug 2024 20:33:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723142030; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zMwc/NJw47FrAO8zT4T3fIs0fG0gqZJfVORzFsOYIrQ=; b=Kv+6UB1v/6/UI8CWsjTYs2lkHclcHbePcBIxLhn4cIEPx4swtcSz5TMudT8OXYgEGgO1Da Cguchb/kAtjmW4Ag0jsiuy9KGYpSWTA979eero3PioX77Cm0gVRWXWF3Rt+0A3RYGk8Roq SWt9wsjoxiDsaTlOCUQiJctZOc9Z5ig= Received: from mail-pf1-f199.google.com (mail-pf1-f199.google.com [209.85.210.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-66-0-kHiCYYMp2j86krg9bLnQ-1; Thu, 08 Aug 2024 14:33:48 -0400 X-MC-Unique: 0-kHiCYYMp2j86krg9bLnQ-1 Received: by mail-pf1-f199.google.com with SMTP id d2e1a72fcca58-710ca162162so1069875b3a.0 for ; Thu, 08 Aug 2024 11:33:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723142027; x=1723746827; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=zMwc/NJw47FrAO8zT4T3fIs0fG0gqZJfVORzFsOYIrQ=; b=Rn/bm9gAwwBGElSS/eC+4Lsa9kgbsaARdBeslFm2XFJ8o89JtiYGkVbxZxzttNi0fP MzvfNuLBTayz5NHuPNaYqWR6+sE1tZIMHIsCGS+DUTIZAjR0Z345yDTBhqOtjuQubDp2 h7Ih1DmLCQRDywLupPpJob6TbMHr9o9TUwYEfXmC5YZjtJCnMiKspkxfLxchxt2/+fWO 2hblOrgsHespIDHgLsUlshq9xogQFVNvuqUC7XLslR6C/hmBdy1iRxR73oLqqg0QPj6u cFrSv3P49GkBlNGUuEiEGefG6nH8i+aKABtdHIVjXMTtbcd+rLHSCJ5qY3ycW4x+ckGI Fc2Q== X-Gm-Message-State: AOJu0YwfzqCGF1I4UMoSHvfSVunFkO/K5Xu55lD9wneCl4z1+d6fT3PF DsVPJ75vo5VdTtLhkP05/aRGDhhW1AuDJmF4SOIO8rubb0h1omFYFiGtWYmOk2dE8V/O4qfLgoO IzIXnd/GQtNzYieOcGTeTOzDEdlmEI3JnxaEKilv2T7p9/zNbyQ== X-Received: by 2002:a05:6a00:2e18:b0:706:7943:b9cc with SMTP id d2e1a72fcca58-710cad6d50dmr3657141b3a.12.1723142027145; Thu, 08 Aug 2024 11:33:47 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGclimYHjTmDxtMkDYcGaECkRhOhUiqzVSAmfjG1I5icbRLsZb0FvQQkIobBbfitLE5aJQfYw== X-Received: by 2002:a05:6a00:2e18:b0:706:7943:b9cc with SMTP id d2e1a72fcca58-710cad6d50dmr3657109b3a.12.1723142026629; Thu, 08 Aug 2024 11:33:46 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-710cb22b418sm1417890b3a.81.2024.08.08.11.33.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Aug 2024 11:33:46 -0700 (PDT) Date: Thu, 8 Aug 2024 20:33:42 +0200 From: Stefano Brivio To: Paul Holzinger Subject: Re: [PATCH v3] conf: Stop parsing options at first non-option argument Message-ID: <20240808203342.5d907e57@elisabeth> In-Reply-To: <20240808121658.6574ef4c@elisabeth> References: <20240808040251.2568850-1-sbrivio@redhat.com> <20240808121658.6574ef4c@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 6WKSEELEPJUSSE32K3HYCNLPXVFAQ3AX X-Message-ID-Hash: 6WKSEELEPJUSSE32K3HYCNLPXVFAQ3AX X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, 8 Aug 2024 12:16:58 +0200 Stefano Brivio wrote: > On Thu, 8 Aug 2024 11:37:38 +0200 > Paul Holzinger wrote: > > > On 08/08/2024 06:02, Stefano Brivio wrote: > > > Given that pasta supports specifying a command to be executed on the > > > command line, even without the usual -- separator as long as there's > > > no ambiguity, we shouldn't eat up options that are not meant for us. > > > > > > Paul reports, for instance, that with: > > > > > > pasta --config-net ip -6 route > > > > > > -6 is taken by pasta to mean --ipv6-only, and we execute 'ip route'. > > > That's because getopt_long(), by default, shuffles the argument list > > > to shift non-option arguments at the end. > > > > > > Avoid that by adding '+' at the beginning of 'optstring'. > > > > > > Reported-by: Paul Holzinger > > > Signed-off-by: Stefano Brivio > > > --- > > > v3: Use '+' in optstring and drop first non-option tracking > > > > > > v2: Instead of overriding 'name' in the getopt_long() loop, to force > > > exiting the loop, adjust the exit condition > > > > > > conf.c | 4 ++-- > > > util.c | 2 +- > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > I like this change but I like to point out that this is a breaking > > change for any user that sets options after the main argument, i.e. pid. > > Oh, right, that actually happens to work, even if we never really > supported that, the man page has only this form: > > pasta [OPTION]... PID > > I could go back to v2, and, on top of that, if we find a single > non-option argument that looks like a PID (a number), we would push it > to the end of argv and continue parsing. > > If we find any other number, including one that's after all the other > options but before the presumed PID we just pushed, we'll report error. > > We have anyway the following problem, which we won't make any worse (it > can't be done without an actual file with POSIX shell, Bash only): > > $ 1() { echo a; } > $ pasta 1; echo > Couldn't open user namespace /proc/1/ns/user: Permission denied > > $ pasta echo; 1 > > a > > > I can tell you that this will not effect podman but I don't know what > > other users exists out there... > > As far as I know, the only other tool using pasta(1) at the moment is > rootless-containers (Docker, Usernetes): > > https://github.com/rootless-containers/rootlesskit/blob/master/pkg/network/pasta/pasta.go > > which is also fine. Other users are developers and people who try out > network topologies and namespaces stuff without root, but I suppose > adding the PID at the end is pretty natural anyway. > > On the other hand, if we can make sure we avoid this kind of breakage > at a small cost, why not. I'll try. ...no, not really a small cost. The logic itself is spectacularly complicated, something on the lines of: case 1: if (c->mode != MODE_PASTA) usage(argv[0], stderr, EXIT_FAILURE); /* There can be just one PID in the middle of options */ errno = 0; strtol(argv[optind - 1], NULL, 10); if (errno) { /* Not a PID, stop parsing here */ name = -1; break; } if (!pid_found) { SWAP(argv[optind - 1], argv[argc - 1]); pid_found = true; } if (optind <= argc) optind--; if (optind == argc) name = -1; break; but this would still be acceptable, I guess. However, we have multiple getopt_long() calls, and we would need to either handle the '1' case for each one of them, or modify optstring, or extract, here, some logic from conf_pasta_ns()... which I would really like to avoid. Let me go ahead with v3, which anyway takes care of all the cases that are documented and we actually meant to support. If somebody really needs to insert a PID in the middle of the option list, we'll "fix" this. -- Stefano