From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: ip6_sysctl(): push netlock down to mrt6_sysctl_mif()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Thu, 24 Jul 2025 23:06:41 +0200

On Thu, Jul 24, 2025 at 11:18:15PM +0300, Vitaliy Makkoveev wrote:
> Except the delivered data, it is the same as mrt_sysctl_vif(). So link
> the network interfaces to the temporary list protected by
> `if_tmplist_lock' rwlock(9) to avoid copyout() with netlock held. Also
> use shared netlock instead of exclusive.

OK bluhm@

> Index: sys/netinet6/ip6_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.285
> diff -u -p -r1.285 ip6_input.c
> --- sys/netinet6/ip6_input.c	24 Jul 2025 18:02:19 -0000	1.285
> +++ sys/netinet6/ip6_input.c	24 Jul 2025 20:13:22 -0000
> @@ -1516,10 +1516,7 @@ ip6_sysctl(int *name, u_int namelen, voi
>  	case IPV6CTL_MRTMIF:
>  		if (newp)
>  			return (EPERM);
> -		NET_LOCK();
> -		error = mrt6_sysctl_mif(oldp, oldlenp);
> -		NET_UNLOCK();
> -		return (error);
> +		return (mrt6_sysctl_mif(oldp, oldlenp));
>  	case IPV6CTL_MRTMFC:
>  		if (newp)
>  			return (EPERM);
> Index: sys/netinet6/ip6_mroute.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_mroute.c,v
> retrieving revision 1.151
> diff -u -p -r1.151 ip6_mroute.c
> --- sys/netinet6/ip6_mroute.c	23 Jul 2025 18:58:38 -0000	1.151
> +++ sys/netinet6/ip6_mroute.c	24 Jul 2025 20:13:22 -0000
> @@ -304,18 +304,36 @@ get_mif6_cnt(struct sioc_mif_req6 *req, 
>  int
>  mrt6_sysctl_mif(void *oldp, size_t *oldlenp)
>  {
> +	TAILQ_HEAD(, ifnet) if_tmplist =
> +	    TAILQ_HEAD_INITIALIZER(if_tmplist);
>  	struct ifnet *ifp;
>  	caddr_t where = oldp;
>  	size_t needed, given;
>  	struct mif6 *mifp;
>  	struct mif6info minfo;
> +	int error = 0;
>  
>  	given = *oldlenp;
>  	needed = 0;
>  	memset(&minfo, 0, sizeof minfo);
> +
> +	rw_enter_write(&if_tmplist_lock);
> +	NET_LOCK_SHARED();
> +
>  	TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
> -		if ((mifp = (struct mif6 *)ifp->if_mcast6) == NULL)
> +		if (ifp->if_mcast6 != NULL) {
> +			if_ref(ifp);
> +			TAILQ_INSERT_TAIL(&if_tmplist, ifp, if_tmplist);
> +		}
> +	}
> +	NET_UNLOCK_SHARED();
> +
> +	TAILQ_FOREACH (ifp, &if_tmplist, if_tmplist) {
> +		NET_LOCK_SHARED();
> +		if ((mifp = (struct mif6 *)ifp->if_mcast6) == NULL) {
> +			NET_UNLOCK_SHARED();
>  			continue;
> +		}
>  
>  		minfo.m6_mifi = mifp->m6_mifi;
>  		minfo.m6_flags = mifp->m6_flags;
> @@ -326,6 +344,7 @@ mrt6_sysctl_mif(void *oldp, size_t *oldl
>  		minfo.m6_bytes_in = mifp->m6_bytes_in;
>  		minfo.m6_bytes_out = mifp->m6_bytes_out;
>  		minfo.m6_rate_limit = mifp->m6_rate_limit;
> +		NET_UNLOCK_SHARED();
>  
>  		needed += sizeof(minfo);
>  		if (where && needed <= given) {
> @@ -333,10 +352,21 @@ mrt6_sysctl_mif(void *oldp, size_t *oldl
>  
>  			error = copyout(&minfo, where, sizeof(minfo));
>  			if (error)
> -				return (error);
> +				break;
>  			where += sizeof(minfo);
>  		}
>  	}
> +
> +	while ((ifp = TAILQ_FIRST(&if_tmplist))) {
> +		TAILQ_REMOVE(&if_tmplist, ifp, if_tmplist);
> +		if_put(ifp);
> +	}
> +
> +	rw_exit_write(&if_tmplist_lock);
> +
> +	if (error)
> +		return (error);
> +
>  	if (where) {
>  		*oldlenp = needed;
>  		if (given < needed)