You can crash a system by running something like: for i in 1 2 3; do while true; do ifconfig bridge0 create& ifconfig bridge0 destroy& done& done
This works with every type of interface I've tried. It appears that if_clone_destroy and if_clone_create race with other ioctls, which causes a variety of different UaFs or just general logic errors. One common root cause appears to be that most ifioctl functions use ifunit() to find an interface by name, which traverses if_list. Writes to if_list are protected by a lock, but reads are apparently unprotected. There's also the question of the life time of the object returned from ifunit(). Most things that access &ifnet's if_list are done without locking, and even if those accesses were to be locked, the lock would be released before the object is no longer used, causing the UaF in that case as well. This patch fixes the issue by making if_clone_{create,destroy} exclusive with all other ifioctls. --- sys/net/if.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git sys/net/if.c sys/net/if.c index fb2f86f4a7c..9eedea83d32 100644 --- sys/net/if.c +++ sys/net/if.c @@ -246,6 +246,11 @@ struct task if_input_task_locked = TASK_INITIALIZER(if_netisr, NULL); */ struct rwlock netlock = RWLOCK_INITIALIZER("netlock"); +/* + * Protect modifications to objects owned by ifnet's if_list + */ +struct rwlock iflist_obj_lock = RWLOCK_INITIALIZER("iflist_obj_lock"); + /* * Network interface utility routines. */ @@ -1905,7 +1910,7 @@ if_setrdomain(struct ifnet *ifp, int rdomain) * Interface ioctls. */ int -ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) +ifioctl_unlocked(struct socket *so, u_long cmd, caddr_t data, struct proc *p) { struct ifnet *ifp; struct ifreq *ifr = (struct ifreq *)data; @@ -2432,6 +2437,35 @@ ifioctl_get(u_long cmd, caddr_t data) return (error); } +int +ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) +{ + int ret; + + switch (cmd) { + case SIOCIFCREATE: + case SIOCIFDESTROY: + rw_enter_write(&iflist_obj_lock); + break; + default: + rw_enter_read(&iflist_obj_lock); + } + + ret = ifioctl_unlocked(so, cmd, data, p); + + switch (cmd) { + case SIOCIFCREATE: + case SIOCIFDESTROY: + rw_exit_write(&iflist_obj_lock); + break; + default: + rw_exit_read(&iflist_obj_lock); + } + + return (ret); +} + + static int if_sffpage_check(const caddr_t data) { -- 2.27.0