Re: git: f5a365e51fee - main - inet6: protect address manipulation with a lock

On 3/31/23, Gleb Smirnoff <glebius@freebsd.org> wrote:
>   Mateusz,
>
> On Thu, Mar 30, 2023 at 08:47:01AM +0000, Mateusz Guzik wrote:
> M> diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
> M> index 27dc3550177c..3d967e9a40c7 100644
> M> --- a/sys/netinet6/in6.c
> M> +++ b/sys/netinet6/in6.c
> M> @@ -168,6 +168,9 @@ static void in6_leave_proxy_ndp_mc(struct ifnet *,
> const struct in6_addr *);
> M>  #define ifa2ia6(ifa)	((struct in6_ifaddr *)(ifa))
> M>  #define ia62ifa(ia6)	(&((ia6)->ia_ifa))
> M>
> M> +static struct sx in6_control_sx;
> M> +SX_SYSINIT(in6_control_sx, &in6_control_sx, "in6_control");
> M> +
> M>  void
> M>  in6_newaddrmsg(struct in6_ifaddr *ia, int cmd)
> M>  {
> M> @@ -254,6 +257,7 @@ in6_control(struct socket *so, u_long cmd, void
> *data,
> M>  	struct	in6_aliasreq *ifra = (struct in6_aliasreq *)data;
> M>  	struct sockaddr_in6 *sa6;
> M>  	int error;
> M> +	bool control_locked = false;
> M>
> M>  	/*
> M>  	 * Compat to make pre-10.x ifconfig(8) operable.
> M> @@ -411,6 +415,8 @@ in6_control(struct socket *so, u_long cmd, void
> *data,
> M>  		if (td != NULL && (error = prison_check_ip6(td->td_ucred,
> M>  		    &sa6->sin6_addr)) != 0)
> M>  			return (error);
> M> +		sx_xlock(&in6_control_sx);
> M> +		control_locked = true;
> M>  		ia = in6ifa_ifpwithaddr(ifp, &sa6->sin6_addr);
> M>  	} else
> M>  		ia = NULL;
> M> @@ -582,6 +588,9 @@ in6_control(struct socket *so, u_long cmd, void
> *data,
> M>
> M>  	error = 0;
> M>  out:
> M> +	if (control_locked)
> M> +		sx_xunlock(&in6_control_sx);
> M> +
> M>  	if (ia != NULL)
> M>  		ifa_free(&ia->ia_ifa);
> M>  	return (error);
>
> Why do you prefer a bool over using sx_xlocked()?
>

It does not read from the lock

but now that you mention it, perhaps i should assert it is not taken
in the else clause

-- 
Mateusz Guzik <mjguzik gmail.com>