Re: kern/54434: ifconfig lo0 destroy leaves lo0 in unusable state

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

The following reply was made to PR kern/54434; it has been noted by GNATS.

From: Ryota Ozaki <ozaki-r%netbsd.org@localhost>
To: logix%foobar.franken.de@localhost
Cc: "gnats-bugs%NetBSD.org@localhost" <gnats-bugs%netbsd.org@localhost>, kern-bug-people%netbsd.org@localhost, 
	gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Subject: Re: kern/54434: ifconfig lo0 destroy leaves lo0 in unusable state
Date: Thu, 15 Aug 2019 13:26:16 +0900

 On Sun, Aug 4, 2019 at 4:08 AM <logix%foobar.franken.de@localhost> wrote:
 >
 > >Number:         54434
 > >Category:       kern
 > >Synopsis:       ifconfig lo0 destroy leaves lo0 in unusable state
 > >Confidential:   no
 > >Severity:       serious
 > >Priority:       medium
 > >Responsible:    kern-bug-people
 > >State:          open
 > >Class:          sw-bug
 > >Submitter-Id:   net
 > >Arrival-Date:   Sat Aug 03 10:35:00 +0000 2019
 > >Originator:     Harold Gutch
 > >Release:        HEAD
 > >Organization:
 > >Environment:
 > NetBSD localhost 9.99.2 NetBSD 9.99.2 (GENERIC) #0: Thu Aug  1 22:23:16 U=
 TC 2019  mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/amd64/compile/GENERIC=
  amd64
 > >Description:
 > When calling "ifconfig <device> destroy", the generic clone destroy funct=
 ion sets the interface ioctl to if_nullioctl before running the device spec=
 ific clone destroy function.  If the device specific destroy function fails=
 , the device still exists and works but there is no reference anymore to it=
 s ioctl function and the device subsequently cannot be configured anymore u=
 ntil the next reboot.
 >
 > This can happen for lo0, ppp*, sl*, srt*, strip*.
 >
 > This was introduced in rev. 1.336 of src/sys/net/if.c.
 > >How-To-Repeat:
 > localhost# ifconfig -a
 > pcn0: flags=3D0x8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
 >         address: 08:00:27:a8:b0:5a
 >         inet 192.168.56.101/24 broadcast 192.168.56.255 flags 0x0
 >         inet6 fe80::a00:27ff:fea8:b05a%pcn0/64 flags 0x0 scopeid 0x1
 > lo0: flags=3D0x8049<UP,LOOPBACK,RUNNING,MULTICAST> mtu 33624
 >         inet 127.0.0.1/8 flags 0x0
 >         inet6 ::1/128 flags 0x20<NODAD>
 >         inet6 fe80::1%lo0/64 flags 0x0 scopeid 0x2
 > localhost# ifconfig lo0 destroy
 > ifconfig: clone_command: Operation not permitted
 > ifconfig: exec_matches: Operation not permitted
 > localhost# ifconfig -a
 > pcn0: flags=3D0x8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
 >         address: 08:00:27:a8:b0:5a
 >         inet 192.168.56.101/24 broadcast 192.168.56.255 flags 0x0
 >         inet6 fe80::a00:27ff:fea8:b05a%pcn0/64 flags 0x0 scopeid 0x1
 > ifconfig: status: getifinfo: Device not configured
 > localhost# ifconfig lo0
 > ifconfig: SIOCGIFFLAGS lo0: Device not configured
 > localhost# ifconfig -l
 > pcn0 lo0
 > >Fix:
 > This patch seems to fix the issue, but I'm not sure if there are cases wh=
 ere it has side effects.
 >
 > --- src/sys/net/if.c.orig       2019-07-25 07:45:57.000000000 +0000
 > +++ src/sys/net/if.c    2019-08-03 12:15:43.318986910 +0000
 > @@ -1611,6 +1611,8 @@
 >         struct if_clone *ifc;
 >         struct ifnet *ifp;
 >         struct psref psref;
 > +       int errno;
 > +       int (*if_ioctl)(struct ifnet *, u_long, void *);
 >
 >         KASSERT(mutex_owned(&if_clone_mtx));
 >
 > @@ -1627,6 +1629,7 @@
 >
 >         /* We have to disable ioctls here */
 >         IFNET_LOCK(ifp);
 > +       if_ioctl =3D ifp->if_ioctl;
 >         ifp->if_ioctl =3D if_nullioctl;
 >         IFNET_UNLOCK(ifp);
 >
 > @@ -1636,7 +1639,13 @@
 >          */
 >         if_put(ifp, &psref);
 >
 > -       return (*ifc->ifc_destroy)(ifp);
 > +       errno =3D (*ifc->ifc_destroy)(ifp);
 > +       if (errno) {
 > +               IFNET_LOCK(ifp);
 > +               ifp->if_ioctl =3D if_ioctl;
 > +               IFNET_UNLOCK(ifp);
 > +       }
 > +       return errno;
 >  }
 >
 >  static bool
 >
 
 Thank you for the fix!  I've committed it (with minor tweaks)
 and will pull it up to NetBSD 8 and 9.
 
   ozaki-r