On Thu, Nov 13, 2025 at 09:44:46PM +0300, Vitaliy Makkoveev wrote:
> Link interface and address descriptors to the `if_tmplist_lock'
> protected temporary lists while holding netlock and then release it. Do
> copyout() while following these lists and holding only
> `if_tmplist_lock'.
> 
> The pagefault handler releases exclusive netlock while copyout(9) needs
> to load pages from swap. So in the cases like ifconf() netlock protected
> lists could became inconsistent. Since we use shared netlock, pagefault
> handler leaves it held, but this breaks swap on NFS. I doubt someone
> uses swap on NFS, but we hold netlock for unpredictable time.
> 
> ok?

I would prefer to have the cleanup cases in one block.  Instead of

		...
> +	}
> +
> +	while ((ifp = TAILQ_FIRST(&if_tmplist))) {
> +		TAILQ_REMOVE(&if_tmplist, ifp, if_tmplist);
> +		if_put(ifp);
>  	}
> -out:
> +	rw_exit_write(&if_tmplist_lock);
> +
>  	ifc->ifc_len -= space;
>  	return (0);
> +
> +free_addrlist:
> +	while((ifa = TAILQ_FIRST(&addr_tmplist))) {
> +		TAILQ_REMOVE(&addr_tmplist, ifa, ifa_tmplist);
> +		ifafree(ifa);
> +	}
> +free_iflist:
> +	while ((ifp = TAILQ_FIRST(&if_tmplist))) {
> +		TAILQ_REMOVE(&if_tmplist, ifp, if_tmplist);
> +		if_put(ifp);
> +	}
> +	rw_exit_write(&if_tmplist_lock);
> +
> +	if (error == 0)
> +		ifc->ifc_len -= space;
> +	return (error);
>  }


I would have written

		...
	}
 free:
	while((ifa = TAILQ_FIRST(&addr_tmplist))) {
		TAILQ_REMOVE(&addr_tmplist, ifa, ifa_tmplist);
		ifafree(ifa);
	}
	while ((ifp = TAILQ_FIRST(&if_tmplist))) {
		TAILQ_REMOVE(&if_tmplist, ifp, if_tmplist);
		if_put(ifp);
	}
	rw_exit_write(&if_tmplist_lock);

	if (error == 0)
		ifc->ifc_len -= space;
	return (error);
}

In the commomn case we do useless free and error checks.  But it
is only 3 comaprisons.  And we do not have two exit paths.  Everything
is always double checked and freed.  There is only one kind of goto
free and you don't have to care to get the right one.

anyway OK bluhm@