The subroutine sl_compress_init(comp, max_state) in slcompress.c at least implies, that the state table can be changed to any value dynamically if called with some appropriate value for max_state. This is not true, because the corresponding table is initialized by the hardcoded MAX_STATES value in the header file slcompress.h. Calling sl_compress_init() with a value greater than MAX_STATES will cause cause writing outside the slcompress structure - bad things happen. sl_compress_init() is currently used like this in the kernel PPP driver, if_ppp.c: #ifdef VJC case PPPIOCSMAXCID: if (error = suser(p->p_ucred, &p->p_acflag)) return (error); s = splnet(); sl_compress_init(&sc->sc_comp, *(int *)data); splx(s); break; #endif in this case, if this ioctl routine is ever issued with a value other than -1 or with a value greater MAX_STATES, random writes into other unknown data structures will occur! Fix: Immediately disable the usage of any parameter for max_state other than -1 now! For the future, make the tstate and rstate structures in struct slcompress resize dynamically so sl_compress_init() is able to do what it should be able to do. How-To-Repeat: Call sl_compress_init() with a max_state value of i.e. 64. (Caution: make a backup before doing this!).
> > >Number: 7556 > >Category: kern > >Synopsis: sl_compress_init() will fail if called anything else than -1 or >MAX_STATE [.....] If anyone picks this up (I haven't the time to be involved with pppd), there's an additional problem when a number of states is negotiated that != MAX_STATES. Namely, it's possible that the peer may agree on (say) 8 states, then proceed to send a header with a slot id of (say) 10. The end result is that a zero'd slot entry is ``adjusted'' by the VJ deltas and will most likely cause a stack scribble. We all know what happens to this in kernel mode :-/ This has been fixed in src/usr.sbin/ppp/slcompress.c - but I don't know how compatible the sources are. -- Brian <brian@Awfulhak.org>, <brian@FreeBSD.org>, <brian@OpenBSD.org> <http://www.Awfulhak.org> Don't _EVER_ lose your sense of humour....
State Changed From-To: open->suspended awaiting patch & committer
Responsible Changed From-To: freebsd-bugs->hm hm is a committer, let him decide whether his PR is still valid
Responsible Changed From-To: hm->freebsd-bugs Reassign to pool; maintainer is away from FreeBSD work due to press of other issues these days.
sl(4) was removed from FreeBSD before 8.0-RELEASE.
This function is used in ng_vjc(4) and sppp(4). The issue described in this bug is still exist.
sppp(4) removed in: commit 6aae3517ed2500fb963ba0a4264b4756088dd0f4 Author: Gleb Smirnoff <glebius@FreeBSD.org> Date: Wed Oct 20 21:08:13 2021 -0700 Retire synchronous PPP kernel driver sppp(4). The last two drivers that required sppp are cp(4) and ce(4). These devices are still produced and can be purchased at Cronyx <http://cronyx.ru/hardware/wan.html>. Since Roman Kurakin <rik@FreeBSD.org> has quit them, they no longer support FreeBSD officially. Later they have dropped support for Linux drivers to. As of mid-2020 they don't even have a developer to maintain their Windows driver. However, their support verbally told me that they could provide aid to a FreeBSD developer with documentaion in case if there appears a new customer for their devices. These drivers have a feature to not use sppp(4) and create an interface, but instead expose the device as netgraph(4) node. Then, you can attach ng_ppp(4) with help of ports/net/mpd5 on top of the node and get your synchronous PPP. Alternatively you can attach ng_frame_relay(4) or ng_cisco(4) for HDLC. Actually, last time I used cp(4) back in 2004, using netgraph(4) instead of sppp(4) was already the right way to do. Thus, remove the sppp(4) related part of the drivers and enable by default the negraph(4) part. Further maintenance of these drivers in the tree shouldn't be a big deal. While doing that, remove some cruft and enable cp(4) compilation on amd64. The ce(4) for some unknown reason marks its internal DDK functions with __attribute__ fastcall, which most likely is safe to remove, but without hardware I'm not going to do that, so ce(4) remains i386-only. Reviewed by: emaste, imp, donner Differential Revision: https://reviews.freebsd.org/D32590 See also: https://reviews.freebsd.org/D23928
^Triage: the 12 branch is now out of support.