Re: protect pmf from network drivers that don't provide if_stop

[Jared McNeill <jmcneill%invisible.ca@localhost>]

Not really a fan of this as it doesn't protect other potential if_stop 
users (and "temporary fix" rarely is..). How about something like this 
instead?


--- sys/net/if.c	29 Jun 2021 21:19:58 -0000	1.486
+++ sys/net/if.c	1 Jul 2021 09:46:10 -0000
@@ -761,11 +761,13 @@ void
 if_register(ifnet_t *ifp)
 {
 	/*
-	 * If the driver has not supplied its own if_ioctl, then
-	 * supply the default.
+	 * If the driver has not supplied its own if_ioctl or if_stop,
+	 * then supply the default.
 	 */
 	if (ifp->if_ioctl == NULL)
 		ifp->if_ioctl = ifioctl_common;
+	if (ifp->if_stop == NULL)
+		ifp->if_stop = if_nullstop;

 	sysctl_sndq_setup(&ifp->if_sysctl_log, ifp->if_xname, &ifp->if_snd);



On Tue, 29 Jun 2021, Brett Lymn wrote:

> Folks,
>
> I turned up a fix I had put into my source tree a while back, I think at
> the time the wireless driver (urtwn IIRC) did not set an entry for
> if_stop.  This resulted in a kernel panic if you tried to sleep the
> machine pmf_class_network_suspend would try to call a null if_stop.
>
> I crafted the following to preserve my sanity, ok to commit?
>
>
> Index: kern_pmf.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/kern_pmf.c,v
> retrieving revision 1.45
> diff -u -r1.45 kern_pmf.c
> --- kern_pmf.c	11 Jun 2020 02:30:21 -0000	1.45
> +++ kern_pmf.c	29 Jun 2021 01:27:01 -0000
> @@ -892,11 +892,18 @@
> 	struct ifnet *ifp = device_pmf_class_private(dev);
> 	int s;
>
> -	s = splnet();
> -	IFNET_LOCK(ifp);
> -	(*ifp->if_stop)(ifp, 0);
> -	IFNET_UNLOCK(ifp);
> -	splx(s);
> +	if (ifp == NULL)
> +		return true;
> +
> +	if ((*ifp->if_stop) == NULL)
> +		printf("device %s has no if_stop\n", ifp->if_xname);
> +	else {
> +		s = splnet();
> +		IFNET_LOCK(ifp);
> +		(*ifp->if_stop)(ifp, 0);
> +		IFNET_UNLOCK(ifp);
> +		splx(s);
> +	}
>
> 	return true;
> }
>
> --
> Brett Lymn
> --
> Sent from my NetBSD device.
>
> "We are were wolves",
> "You mean werewolves?",
> "No we were wolves, now we are something else entirely",
> "Oh"