From nobody Thu Jan 19 17:11:38 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4NyTgr0LNNz2v8R1 for ; Thu, 19 Jan 2023 17:11:52 +0000 (UTC) (envelope-from asomers@gmail.com) Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4NyTgq5qRMz3wZF; Thu, 19 Jan 2023 17:11:51 +0000 (UTC) (envelope-from asomers@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-vk1-f171.google.com with SMTP id b81so1268221vkf.1; Thu, 19 Jan 2023 09:11:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vPgXUyqNeU0GQCcbupm8EUczJJRgWhf5WQoF55Xi70w=; b=gCCbi6YJsnIK/1DENtyuFzRoucgaYUkWilX9+FnooIUJfhPZUGNZRDFzCsxrvSoSjn mzVUJ7RneDVjEQAlGajca8LTiJX72yGFSsRgJZV6a1GHTJyrn3lB8a2eVrdonCISIYvE Mr9ydatTjhLJh1iye7I93+caOwwtmT7CtWFJNL3w+7KLbS7FzSnBilrerffkeLCb+E3p 1w2guQUt9hM/qJixb7JNrFb/86dgTWJz2SI/jY0YmwPZ2Z0e6/8e+lF652sx3IMDqyqd oISXkbKAuTaC0DAtum2f2MjQPUOBTVjRHZ2+ZKtn6cIs5PXGIMLomRaPEeZwZ+YIaLow Wttg== X-Gm-Message-State: AFqh2krAB2p2irqbxm/l/FeQJ3ZmWQ3BfzXuVqA0dQ7JO9PclwgV8iH9 lx7MjZAhMcXAW+G51GD/tH5ASG53AlMpfqWaiBVuD7Ff4n8= X-Google-Smtp-Source: AMrXdXtFCyc7D502hKhzs+l/AOXqkMmZJ04w4YEy7d6/TmdbTF9HduwQOg4Wbxn4Q+UjGTFKOsG2WMoTZFHdRNCs3bk= X-Received: by 2002:a1f:3055:0:b0:380:5dbd:1076 with SMTP id w82-20020a1f3055000000b003805dbd1076mr1560183vkw.22.1674148310602; Thu, 19 Jan 2023 09:11:50 -0800 (PST) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202301091857.309Iv87L068285@gitrepo.freebsd.org> <2f4e4ccf-b19a-4f8f-a9e0-72298e500d7c@app.fastmail.com> In-Reply-To: <2f4e4ccf-b19a-4f8f-a9e0-72298e500d7c@app.fastmail.com> From: Alan Somers Date: Thu, 19 Jan 2023 10:11:38 -0700 Message-ID: Subject: Re: git: 2c24ad3377a6 - main - ifconfig: abort if loading a module fails other than for ENOENT To: "Danilo G. Baio" Cc: dev-commits-src-all@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4NyTgq5qRMz3wZF X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On Thu, Jan 19, 2023 at 7:03 AM Danilo G. Baio wrote: > > > > On Mon, Jan 9, 2023, at 15:57, Alan Somers wrote: > > The branch main has been updated by asomers: > > > > URL: > > https://cgit.FreeBSD.org/src/commit/?id=2c24ad3377a6f584e484656db8390e4eb7cfc119 > > > > commit 2c24ad3377a6f584e484656db8390e4eb7cfc119 > > Author: Alan Somers > > AuthorDate: 2022-12-26 02:06:21 +0000 > > Commit: Alan Somers > > CommitDate: 2023-01-10 02:56:18 +0000 > > > > ifconfig: abort if loading a module fails other than for ENOENT > > > > If "ifconfig create" tries to load a kernel module, and the module > > exists but can't be loaded, fail the command with a useful error > > message. This is helpful, for example, when trying to create a cloned > > interface in a vnet jail. But ignore ENOENT, because sometimes ifconfig > > can't correctly guess the name of the required kernel module. > > > > MFC after: 2 weeks > > Reviewed by: jhb > > Differential Revision: https://reviews.freebsd.org/D37873 > > --- > > sbin/ifconfig/ifconfig.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/sbin/ifconfig/ifconfig.c b/sbin/ifconfig/ifconfig.c > > index 462d543125c4..120207a6927e 100644 > > --- a/sbin/ifconfig/ifconfig.c > > +++ b/sbin/ifconfig/ifconfig.c > > @@ -1719,11 +1719,19 @@ ifmaybeload(const char *name) > > } > > } > > > > - /* > > - * Try to load the module. But ignore failures, because ifconfig can't > > - * infer the names of all drivers (eg mlx4en(4)). > > - */ > > - (void) kldload(ifkind); > > + /* Try to load the module. */ > > + if (kldload(ifkind) < 0) { > > + switch (errno) { > > + case ENOENT: > > + /* > > + * Ignore ENOENT, because ifconfig can't infer the > > + * names of all drivers (eg mlx4en(4)). > > + */ > > + break; > > + default: > > + err(1, "kldload(%s)", ifkind); > > + } > > + } > > } > > > > static struct cmd basic_cmds[] = { > > > Hi. > > I have a jail with vnet where the interface is renamed that stopped working after this. > > from inside the jail: > > root@test:/ # ifconfig > lo0: flags=8049 metric 0 mtu 16384 > options=680003 > inet6 ::1 prefixlen 128 > inet6 fe80::1%lo0 prefixlen 64 scopeid 0x10 > inet 127.0.0.1 netmask 0xff000000 > groups: lo > nd6 options=21 > vnet0b_test: flags=8862 metric 0 mtu 1500 > options=8 > ether 02:27:72:a7:28:0b > groups: epair > media: Ethernet 10Gbase-T (10Gbase-T ) > status: active > nd6 options=29 > > root@test:/ # ifconfig vnet0b_test > ifconfig: kldload(if_vnet): Operation not permitted > > > If I don't rename the interface, that works. > > jail.conf: > > test { > vnet; > $index = "0"; > vnet.interface = "vnet${index}b_${name}"; > exec.prestart += "ifconfig epair${index} create"; > exec.prestart += "ifconfig ${bridge} addm epair${index}a"; > exec.prestart += "ifconfig epair${index}a up name vnet${index}a_${name}"; > exec.prestart += "ifconfig epair${index}b up name vnet${index}b_${name}"; > exec.poststop += "ifconfig ${bridge} deletem vnet${index}a_${name}"; > exec.poststop += "ifconfig vnet${index}a_${name} destroy"; > devfs_ruleset = "11"; # add path 'bpf*' unhide (devfs.rules) > } > > That's a bit odd, I know, could be using description instead. > > Just reporting. > > Regards. > -- > Danilo G. Baio Ugh, it looks like kldload(2) is doing the privilege check before the file existence check. I'm not sure of the best solution: * Change kern_kldload to check for file existence first. This would ring some alarm bells among security folks, and it isn't totally easy to do, either. * Change ifconfig(8) to do an existence check of its own. This would be ugly. * Change ifconfig(8) so that it doesn't attempt to load modules when just listing an interface. This might be incomplete, but is probably worth doing anyway.