On Thu, Jan 23, 2025 at 08:15:25PM +0300, Vitaliy Makkoveev wrote:
> Netlock required only to store data to local variable, the rest could be
> done lockless. sysctl_source() is local to net/rtsock.c.

struct sockaddr sa is not enough to store all adreses families
sa = *sa_tmp truncates IPv6 address
copyout(&sa, ..., size) leaks kernel stack memory to userland

bluhm

> Index: sys/net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.377
> diff -u -p -r1.377 rtsock.c
> --- sys/net/rtsock.c	9 Jan 2025 18:20:29 -0000	1.377
> +++ sys/net/rtsock.c	23 Jan 2025 17:07:28 -0000
> @@ -2136,12 +2136,17 @@ sysctl_ifnames(struct walkarg *w)
>  int
>  sysctl_source(int af, u_int tableid, struct walkarg *w)
>  {
> -	struct sockaddr	*sa;
> +	struct sockaddr	 sa;
> +	struct sockaddr	*sa_tmp;
>  	int		 size, error = 0;
>  
> -	sa = rtable_getsource(tableid, af);
> -	if (sa) {
> -		switch (sa->sa_family) {
> +	NET_LOCK_SHARED();
> +	if ((sa_tmp = rtable_getsource(tableid, af)) != NULL)
> +		sa = *sa_tmp;
> +	NET_UNLOCK_SHARED();
> +
> +	if (sa_tmp) {
> +		switch (sa.sa_family) {
>  		case AF_INET:
>  			size = sizeof(struct sockaddr_in);
>  			break;
> @@ -2155,7 +2160,7 @@ sysctl_source(int af, u_int tableid, str
>  		}
>  		w->w_needed += size;
>  		if (w->w_where && w->w_needed <= w->w_given) {
> -			if ((error = copyout(sa, w->w_where, size)))
> +			if ((error = copyout(&sa, w->w_where, size)))
>  				return (error);
>  			w->w_where += size;
>  		}
> @@ -2236,7 +2241,6 @@ sysctl_rtable(int *name, u_int namelen, 
>  		tableid = w.w_arg;
>  		if (!rtable_exists(tableid))
>  			return (ENOENT);
> -		NET_LOCK_SHARED();
>  		for (i = 1; i <= AF_MAX; i++) {
>  			if (af != 0 && af != i)
>  				continue;
> @@ -2247,7 +2251,6 @@ sysctl_rtable(int *name, u_int namelen, 
>  			if (error)
>  				break;
>  		}
> -		NET_UNLOCK_SHARED();
>  		break;
>  	}
>  	free(w.w_tmem, M_RTABLE, w.w_tmemsize);