Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 19 Jan 2023 10:11:38 -0700
From:      Alan Somers <asomers@freebsd.org>
To:        "Danilo G. Baio" <dbaio@freebsd.org>
Cc:        dev-commits-src-all@freebsd.org
Subject:   Re: git: 2c24ad3377a6 - main - ifconfig: abort if loading a module fails other than for ENOENT
Message-ID:  <CAOtMX2hv182P2HTAPkbYDZiwNxkV2-C%2BWp2%2BL0SpfDpqn2Zccw@mail.gmail.com>
In-Reply-To: <2f4e4ccf-b19a-4f8f-a9e0-72298e500d7c@app.fastmail.com>
References:  <202301091857.309Iv87L068285@gitrepo.freebsd.org> <2f4e4ccf-b19a-4f8f-a9e0-72298e500d7c@app.fastmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Jan 19, 2023 at 7:03 AM Danilo G. Baio <dbaio@freebsd.org> 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 <asomers@FreeBSD.org>
> > AuthorDate: 2022-12-26 02:06:21 +0000
> > Commit:     Alan Somers <asomers@FreeBSD.org>
> > 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<UP,LOOPBACK,RUNNING,MULTICAST> metric 0 mtu 16384
>         options=680003<RXCSUM,TXCSUM,LINKSTATE,RXCSUM_IPV6,TXCSUM_IPV6>
>         inet6 ::1 prefixlen 128
>         inet6 fe80::1%lo0 prefixlen 64 scopeid 0x10
>         inet 127.0.0.1 netmask 0xff000000
>         groups: lo
>         nd6 options=21<PERFORMNUD,AUTO_LINKLOCAL>
> vnet0b_test: flags=8862<BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
>         options=8<VLAN_MTU>
>         ether 02:27:72:a7:28:0b
>         groups: epair
>         media: Ethernet 10Gbase-T (10Gbase-T <full-duplex>)
>         status: active
>         nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
>
> 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.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOtMX2hv182P2HTAPkbYDZiwNxkV2-C%2BWp2%2BL0SpfDpqn2Zccw>