From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 670B85A004E for ; Fri, 09 Aug 2024 05:33:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1723174409; bh=2sUsuA2eBGbXZGTWArDHe9GI+CFiem6FO881fMaAlzQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ICpmBAp7FglFSxo3TF4bWFxD09z8bZJqBZGxcRnBa+QhzQRqZxPh3KsetAMZK0zZC qd7Kn9VpCO0J+z9J71q92hXJaZT19+DqvMO7e4cs3CeEIdqFQ/H9mrpBuf+OZ14HdN yDpR7H4yBn82eQK7iNrvHV5N4UjY5dqD1zkic7J39hl+msr4tZ8Xj/ThlUuVNJptW3 mCRVT2aLyFAWCSzoOexdikq/JF1e50+a5owraz4Yh9oPvC3mN5l7p9zLFsvCsxuhQl Ovlmg0f9Wle8GFh8qFN+lIGlHNI9EGZXxYZfUs+mBIzp/HrwG1wNLlaUbDqCYzhN+8 Q60NKb7om4IzA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Wg8dP5yXTz4wp0; Fri, 9 Aug 2024 13:33:29 +1000 (AEST) Date: Fri, 9 Aug 2024 10:47:31 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3] conf: Stop parsing options at first non-option argument Message-ID: References: <20240808040251.2568850-1-sbrivio@redhat.com> <20240808121658.6574ef4c@elisabeth> <20240808203342.5d907e57@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="jYHq5HBPWgQckWRx" Content-Disposition: inline In-Reply-To: <20240808203342.5d907e57@elisabeth> Message-ID-Hash: F66JMTTLJ2FVUAAV3DVCM2KIF5NFXTOE X-Message-ID-Hash: F66JMTTLJ2FVUAAV3DVCM2KIF5NFXTOE X-MailFrom: dgibson@gandalf.ozlabs.org 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: Paul Holzinger , passt-dev@passt.top 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: --jYHq5HBPWgQckWRx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Aug 08, 2024 at 08:33:42PM +0200, Stefano Brivio wrote: > On Thu, 8 Aug 2024 12:16:58 +0200 > Stefano Brivio wrote: >=20 > > On Thu, 8 Aug 2024 11:37:38 +0200 > > Paul Holzinger wrote: > >=20 > > > On 08/08/2024 06:02, Stefano Brivio wrote: =20 > > > > 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(-) > > > > =20 > > > I like this change but I like to point out that this is a breaking=20 > > > change for any user that sets options after the main argument, i.e. p= id. =20 > >=20 > > Oh, right, that actually happens to work, even if we never really > > supported that, the man page has only this form: > >=20 > > pasta [OPTION]... PID > >=20 > > 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. Please no, this sounds horribly complicated. My experience is that when you try to intuit what the user wants this way, you're more likely to just behave unpredictably in other cases. > > 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. > >=20 > > 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): > >=20 > > $ 1() { echo a; } > > $ pasta 1; echo > > Couldn't open user namespace /proc/1/ns/user: Permission denied > > =20 > > $ pasta echo; 1 > > =20 > > a > >=20 > > > I can tell you that this will not effect podman but I don't know what= =20 > > > other users exists out there... =20 > >=20 > > As far as I know, the only other tool using pasta(1) at the moment is > > rootless-containers (Docker, Usernetes): > >=20 > > https://github.com/rootless-containers/rootlesskit/blob/master/pkg/ne= twork/pasta/pasta.go > >=20 > > 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. > >=20 > > On the other hand, if we can make sure we avoid this kind of breakage > > at a small cost, why not. I'll try. >=20 > ...no, not really a small cost. The logic itself is spectacularly > complicated, something on the lines of: >=20 > case 1: > if (c->mode !=3D MODE_PASTA) > usage(argv[0], stderr, EXIT_FAILURE); >=20 > /* There can be just one PID in the middle of options */ > errno =3D 0; > strtol(argv[optind - 1], NULL, 10); > if (errno) { /* Not a PID, stop parsing here */ > name =3D -1; > break; > } >=20 > if (!pid_found) { > SWAP(argv[optind - 1], argv[argc - 1]); > pid_found =3D true; > } >=20 > if (optind <=3D argc) > optind--; >=20 > if (optind =3D=3D argc) > name =3D -1; >=20 > break; >=20 > 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. >=20 > 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. Phew. --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --jYHq5HBPWgQckWRx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAma1ZwkACgkQzQJF27ox 2GcgEBAAiiwU0Zszgh4TwCYxeZ2s9/WVKrMPK7HF+asV+8nzZcizxVpJmI8JRbZQ PUnxqRrscwHWoRjdWoJj7VlxBgObpGpAVvVvsa/mcQ1GJetn9omlhgvb7Cg1fin8 Gc7WK00Ci9Vc3TcN79pMNbrIorE3ZRb334eRSpEaO44rPyu75aoHBNQAnp2Afz7N JHfVAxC+7Y3WobcmXCIBHwPyQjBLvjXmG7i6wqgR7k/hh0QVtXrkixINN/9mOvwV Mwhx4solsDIEVBEj119U2MfPHyg9wHjtylK6NW6eHknlE2qzWgZwUBT+nPvXLwwz iiXqOsoFloCmlk0XgBLltzPMwCVloYzpPDb/7RCGYyish5QPmll6tsVYRCKwQ0Ac K2t6AfTwfJ579GmOMHX3iZYS3yLkLq7z7HL96qh57pODqrK1jMxSkaO8L1ek194b lnoOplcmutRZbhLweGjgXGk8ld/FhqXhVRyb7NZppYEJNgibei7CV0tteCKFED1o /Bt70NgnETSK5XdtYz9GI8IjV8Euz7lElXOw8fkvKWStuFgogQ3dSdPxLqo0m9Qx wA/k1zH2YkmGEaV4gt9LiMJlKjZtJ89jLabRttk1myy4Edgv30ywjJAFWEteFWZz SmUCfhanpEH4YFEiiM1xRnfAVt/zhrGFEY5G7iVgtMTGIIDVouE= =ULNX -----END PGP SIGNATURE----- --jYHq5HBPWgQckWRx--