On Mon, Oct 27, 2025 at 08:54:02PM +0300, Vitaliy Makkoveev wrote:
> Seems the whole sppp_output() should take kernel lock. I have no ability
> to test this diff.
I also came to the conclusion that some big kernel lock is missing.
Just wondering why this bug appears now. We have removed kernel
lock from network stack a long time ago.
OK bluhm@
> Index: sys/net/if_spppsubr.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.198
> diff -u -p -r1.198 if_spppsubr.c
> --- sys/net/if_spppsubr.c 19 Aug 2025 12:34:15 -0000 1.198
> +++ sys/net/if_spppsubr.c 27 Oct 2025 17:51:15 -0000
> @@ -591,6 +591,7 @@ sppp_output(struct ifnet *ifp, struct mb
> }
> #endif
>
> + KERNEL_LOCK();
> s = splnet();
>
> getmicrouptime(&tv);
> @@ -599,8 +600,8 @@ sppp_output(struct ifnet *ifp, struct mb
> if ((ifp->if_flags & IFF_UP) == 0 ||
> (ifp->if_flags & (IFF_RUNNING | IFF_AUTO)) == 0) {
> m_freem (m);
> - splx (s);
> - return (ENETDOWN);
> + rv = ENETDOWN;
> + goto out;
> }
>
> if ((ifp->if_flags & (IFF_RUNNING | IFF_AUTO)) == IFF_AUTO) {
> @@ -635,11 +636,9 @@ sppp_output(struct ifnet *ifp, struct mb
> u_int8_t proto = ip->ip_p;
>
> m_freem(m);
> - splx(s);
> if (proto == IPPROTO_TCP)
> - return (EADDRNOTAVAIL);
> - else
> - return (0);
> + rv = EADDRNOTAVAIL;
> + goto out;
> }
> }
>
> @@ -677,8 +676,8 @@ sppp_output(struct ifnet *ifp, struct mb
> default:
> m_freem(m);
> ++ifp->if_oerrors;
> - splx(s);
> - return (EAFNOSUPPORT);
> + rv = EAFNOSUPPORT;
> + goto out;
> }
>
> M_PREPEND(m, 2, M_DONTWAIT);
> @@ -688,8 +687,9 @@ sppp_output(struct ifnet *ifp, struct mb
> "no memory for transmit header\n",
> SPP_ARGS(ifp));
> ++ifp->if_oerrors;
> - splx(s);
> - return (rv ? rv : ENOBUFS);
> + if (rv == 0)
> + rv = ENOBUFS;
> + goto out;
> }
> *mtod(m, u_int16_t *) = protocol;
>
> @@ -700,8 +700,7 @@ sppp_output(struct ifnet *ifp, struct mb
> rv = if_enqueue(ifp, m);
> if (rv != 0) {
> ifp->if_oerrors++;
> - splx(s);
> - return (rv);
> + goto out;
> }
>
> /*
> @@ -710,9 +709,10 @@ sppp_output(struct ifnet *ifp, struct mb
> * according to RFC 1333.
> */
> ifp->if_obytes += sp->pp_framebytes;
> +out:
> + KERNEL_UNLOCK();
> splx(s);
> -
> - return (0);
> + return (rv);
> }
>
> void
