From nobody Fri Jan 20 11:04:33 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 4NyxTn6b9Cz2tyqN for ; Fri, 20 Jan 2023 11:04:45 +0000 (UTC) (envelope-from melifaro@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4NyxTn5xPtz3KFj; Fri, 20 Jan 2023 11:04:45 +0000 (UTC) (envelope-from melifaro@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1674212685; 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=OuiYCb1ZOtJGwbifDgyW9ghl/QLRIF/opYMoHbTB/Dw=; b=G9YjJstSmX3u/Q5Gz11r3GpqbxC7bjpnivL2xJN3UYebELnLt6YqetdkOWau1pcDxyIGA7 RCvMPRn86IrusP1ZDHeJW+J+oxigUJt/XdY7HPGWnFfz1WIYKvBGBxnpditSdpYNjiZ/7D Qi9ofehjOi39v3sS0L+H8sRET0HfzvWcKqZUK6x/EH3m6khYVKO8afOGVmx2anSETCum/5 B5nL9gu3971bpxWmyTfCPif2jJz3NBTMVgce0xzxXjWDG8BRJlxmX8PS/xUV3Qb74cqv5K Pv5Oki/giwGkxFns+8DCqMWLuESa1QlX2VzT9fHkXzq5L0ymUmkPlI7CkWhPQA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1674212685; 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=OuiYCb1ZOtJGwbifDgyW9ghl/QLRIF/opYMoHbTB/Dw=; b=IN2NUPkWVSU80y1sNyVzuw7c59CQPIAmbCbcOas5LwkZAlbGZfIqbJ/gpfDCkEHQ+O+nVn fhFQTt/n/b61U2GYcyY7uF1InOJ+VxFfSPxAgkTIhgM5wy59CTMYuz/cl66KgPbchzHPSH IEHEtFY71fbzxzarkEFP1KUbkgfEnDSYW0bnKeCmM56mE35nH/DjyaqshISJ0yEbrBqaNZ itwRhxj9QKwRECbtmYEWocTWzafeswhQMt3BBx7TsbIBWHOvFMdNOdATDqPyITCCZzcero 56lA9Bz5CHgClyN+IFmnuAokPPRhNkV1KPGKJGDOBesFALduq2Ld89THHZH9zA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1674212685; a=rsa-sha256; cv=none; b=mL884vwRIy8HfyHRDEJko31wvPR13ZgpQJn3qKDjKxtPRwEQqIyk6LsL/tRxvXTxaKXZQI iCPEGycAGtfo6f0bTqPGp8QemaiKLgvGSMPjIbUp+a2S6YqPFOsMDjP3QlH0Ov3+GbSL0/ 2DVQM2q2ErSDRVAZWVv4BSSHgbUutaZZUiHBoOMlY3txcTiZHZcbjyVCGM6VwubABgRh/R DuCujZzVPILpqkRgb6j+ubZddUPDwoVyzJeYNExmljq4c+pkNkd6mtktdBqR4tw0cQwI6d BI7iyuohXsa/OkG/c1tT3gnEko4YuukE6AoW/mhScguqFdBs3VfheQQpU6ApyA== Received: from smtpclient.apple (unknown [IPv6:2a02:8084:d6bb:510:2daa:c4ca:2eed:fc35]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) (Authenticated sender: melifaro/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4NyxTn21Xqz18LJ; Fri, 20 Jan 2023 11:04:45 +0000 (UTC) (envelope-from melifaro@freebsd.org) Content-Type: text/plain; charset=utf-8 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 (Mac OS X Mail 16.0 \(3731.200.110.1.12\)) Subject: Re: git: 2c24ad3377a6 - main - ifconfig: abort if loading a module fails other than for ENOENT From: Alexander Chernikov In-Reply-To: Date: Fri, 20 Jan 2023 11:04:33 +0000 Cc: "Danilo G. Baio" , dev-commits-src-all@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <1E9FAE83-B5C2-4E1F-8D04-CF4F477F76C7@freebsd.org> References: <202301091857.309Iv87L068285@gitrepo.freebsd.org> <2f4e4ccf-b19a-4f8f-a9e0-72298e500d7c@app.fastmail.com> To: Alan Somers X-Mailer: Apple Mail (2.3731.200.110.1.12) X-ThisMailContainsUnwantedMimeParts: N > On 19 Jan 2023, at 17:11, Alan Somers wrote: >=20 > On Thu, Jan 19, 2023 at 7:03 AM Danilo G. Baio = wrote: >>=20 >>=20 >>=20 >> On Mon, Jan 9, 2023, at 15:57, Alan Somers wrote: >>> The branch main has been updated by asomers: >>>=20 >>> URL: >>> = https://cgit.FreeBSD.org/src/commit/?id=3D2c24ad3377a6f584e484656db8390e4e= b7cfc119 >>>=20 >>> commit 2c24ad3377a6f584e484656db8390e4eb7cfc119 >>> Author: Alan Somers >>> AuthorDate: 2022-12-26 02:06:21 +0000 >>> Commit: Alan Somers >>> CommitDate: 2023-01-10 02:56:18 +0000 >>>=20 >>> ifconfig: abort if loading a module fails other than for ENOENT >>>=20 >>> 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. >>>=20 >>> 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(-) >>>=20 >>> 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) >>> } >>> } >>>=20 >>> - /* >>> - * 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); >>> + } >>> + } >>> } >>>=20 >>> static struct cmd basic_cmds[] =3D { >>=20 >>=20 >> Hi. >>=20 >> I have a jail with vnet where the interface is renamed that stopped = working after this. >>=20 >> from inside the jail: >>=20 >> root@test:/ # ifconfig >> lo0: flags=3D8049 metric 0 mtu 16384 >> options=3D680003= >> inet6 ::1 prefixlen 128 >> inet6 fe80::1%lo0 prefixlen 64 scopeid 0x10 >> inet 127.0.0.1 netmask 0xff000000 >> groups: lo >> nd6 options=3D21 >> vnet0b_test: flags=3D8862 metric = 0 mtu 1500 >> options=3D8 >> ether 02:27:72:a7:28:0b >> groups: epair >> media: Ethernet 10Gbase-T (10Gbase-T ) >> status: active >> nd6 options=3D29 >>=20 >> root@test:/ # ifconfig vnet0b_test >> ifconfig: kldload(if_vnet): Operation not permitted >>=20 >>=20 >> If I don't rename the interface, that works. >>=20 >> jail.conf: >>=20 >> test { >> vnet; >> $index =3D "0"; >> vnet.interface =3D "vnet${index}b_${name}"; >> exec.prestart +=3D "ifconfig epair${index} create"; >> exec.prestart +=3D "ifconfig ${bridge} addm epair${index}a"; >> exec.prestart +=3D "ifconfig epair${index}a up name = vnet${index}a_${name}"; >> exec.prestart +=3D "ifconfig epair${index}b up name = vnet${index}b_${name}"; >> exec.poststop +=3D "ifconfig ${bridge} deletem = vnet${index}a_${name}"; >> exec.poststop +=3D "ifconfig vnet${index}a_${name} destroy"; >> devfs_ruleset =3D "11"; # add path 'bpf*' unhide (devfs.rules) >> } >>=20 >> That's a bit odd, I know, could be using description instead. >>=20 >> Just reporting. >>=20 >> Regards. >> -- >> Danilo G. Baio >=20 > 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. I think another question is that if if should be done by ifconfig(8) at = all. Kernel can take care of trying to load the required modules, = checking the privileges. I=E2=80=99m considering adding such code for the netlink-based interface = creation. >=20