From: Alexander Bluhm <alexander.bluhm@gmx.net>
Subject: Re: const sockaddr in tcp syn cache
To: tech@openbsd.org
Date: Thu, 25 Jan 2024 00:47:37 +0100

On Fri, Jan 19, 2024 at 03:01:12PM +0100, Alexander Bluhm wrote:
> On Fri, Jan 19, 2024 at 02:47:53PM +0100, Alexander Bluhm wrote:
> > tcp6_ctlinput() casts a constant sockaddr_sin6 to non-const sockaddr.
> > sa6_src may be &sa6_any which lives in read-only data section.
> >
> > Better pass down the const addresses to tcp syn cache.  We only need
> > them for lookup.
> 
> Oops, missed one satosin_const which breaks non-INET6 builds.
> 
> updated diff, ok?

anyone?

> Index: netinet/tcp_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
> diff -u -p -r1.398 tcp_input.c
> --- netinet/tcp_input.c	11 Jan 2024 13:49:49 -0000	1.398
> +++ netinet/tcp_input.c	19 Jan 2024 13:56:41 -0000
> @@ -201,8 +201,8 @@ int	 syn_cache_add(struct sockaddr *, st
>  struct socket *syn_cache_get(struct sockaddr *, struct sockaddr *,
>  		struct tcphdr *, unsigned int, unsigned int, struct socket *,
>  		struct mbuf *, uint64_t);
> -struct syn_cache *syn_cache_lookup(struct sockaddr *, struct sockaddr *,
> -		struct syn_cache_head **, u_int);
> +struct syn_cache *syn_cache_lookup(const struct sockaddr *,
> +		const struct sockaddr *, struct syn_cache_head **, u_int);
> 
>  /*
>   * Insert segment ti into reassembly queue of tcp with
> @@ -3109,9 +3109,9 @@ struct mutex syn_cache_mtx = MUTEX_INITI
>  #ifndef INET6
>  #define	SYN_HASHALL(hash, src, dst, rand) \
>  do {									\
> -	hash = SYN_HASH(&satosin(src)->sin_addr,			\
> -		satosin(src)->sin_port,					\
> -		satosin(dst)->sin_port, (rand));			\
> +	hash = SYN_HASH(&satosin_const(src)->sin_addr,			\
> +		satosin_const(src)->sin_port,				\
> +		satosin_const(dst)->sin_port, (rand));			\
>  } while (/*CONSTCOND*/ 0)
>  #else
>  #define SYN_HASH6(sa, sp, dp, rand) \
> @@ -3125,14 +3125,14 @@ do {									\
>  do {									\
>  	switch ((src)->sa_family) {					\
>  	case AF_INET:							\
> -		hash = SYN_HASH(&satosin(src)->sin_addr,		\
> -			satosin(src)->sin_port,				\
> -			satosin(dst)->sin_port, (rand));		\
> +		hash = SYN_HASH(&satosin_const(src)->sin_addr,		\
> +			satosin_const(src)->sin_port,			\
> +			satosin_const(dst)->sin_port, (rand));		\
>  		break;							\
>  	case AF_INET6:							\
> -		hash = SYN_HASH6(&satosin6(src)->sin6_addr,		\
> -			satosin6(src)->sin6_port,			\
> -			satosin6(dst)->sin6_port, (rand));		\
> +		hash = SYN_HASH6(&satosin6_const(src)->sin6_addr,	\
> +			satosin6_const(src)->sin6_port,			\
> +			satosin6_const(dst)->sin6_port, (rand));	\
>  		break;							\
>  	default:							\
>  		hash = 0;						\
> @@ -3423,7 +3423,7 @@ syn_cache_cleanup(struct tcpcb *tp)
>   * Find an entry in the syn cache.
>   */
>  struct syn_cache *
> -syn_cache_lookup(struct sockaddr *src, struct sockaddr *dst,
> +syn_cache_lookup(const struct sockaddr *src, const struct sockaddr *dst,
>      struct syn_cache_head **headp, u_int rtableid)
>  {
>  	struct syn_cache_set *sets[2];
> @@ -3709,8 +3709,8 @@ syn_cache_reset(struct sockaddr *src, st
>  }
> 
>  void
> -syn_cache_unreach(struct sockaddr *src, struct sockaddr *dst, struct tcphdr *th,
> -    u_int rtableid)
> +syn_cache_unreach(const struct sockaddr *src, const struct sockaddr *dst,
> +    struct tcphdr *th, u_int rtableid)
>  {
>  	struct syn_cache *sc;
>  	struct syn_cache_head *scp;
> Index: netinet/tcp_subr.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_subr.c,v
> diff -u -p -r1.195 tcp_subr.c
> --- netinet/tcp_subr.c	11 Jan 2024 13:49:49 -0000	1.195
> +++ netinet/tcp_subr.c	19 Jan 2024 13:37:29 -0000
> @@ -698,8 +698,8 @@ tcp6_ctlinput(int cmd, struct sockaddr *
>  		} else if (inet6ctlerrmap[cmd] == EHOSTUNREACH ||
>  		    inet6ctlerrmap[cmd] == ENETUNREACH ||
>  		    inet6ctlerrmap[cmd] == EHOSTDOWN)
> -			syn_cache_unreach((struct sockaddr *)sa6_src,
> -			    sa, &th, rdomain);
> +			syn_cache_unreach(sin6tosa_const(sa6_src), sa, &th,
> +			    rdomain);
>  		in_pcbunref(inp);
>  	} else {
>  		in6_pcbnotify(&tcbtable, sa6, 0,
> Index: netinet/tcp_var.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v
> diff -u -p -r1.174 tcp_var.h
> --- netinet/tcp_var.h	11 Jan 2024 13:49:49 -0000	1.174
> +++ netinet/tcp_var.h	19 Jan 2024 13:37:29 -0000
> @@ -799,7 +799,7 @@ int	tcp_signature(struct tdb *, int, str
>  #endif /* TCP_SIGNATURE */
>  void     tcp_set_iss_tsm(struct tcpcb *);
> 
> -void	 syn_cache_unreach(struct sockaddr *, struct sockaddr *,
> +void	 syn_cache_unreach(const struct sockaddr *, const struct sockaddr *,
>  	   struct tcphdr *, u_int);
>  void	 syn_cache_init(void);
>  void	 syn_cache_cleanup(struct tcpcb *);
> Index: netinet6/in6.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.h,v
> diff -u -p -r1.111 in6.h
> --- netinet6/in6.h	28 Nov 2023 13:23:20 -0000	1.111
> +++ netinet6/in6.h	19 Jan 2024 13:37:29 -0000
> @@ -459,6 +459,12 @@ sin6tosa(struct sockaddr_in6 *sin6)
>  	return ((struct sockaddr *)(sin6));
>  }
> 
> +static inline const struct sockaddr *
> +sin6tosa_const(const struct sockaddr_in6 *sin6)
> +{
> +	return ((const struct sockaddr *)(sin6));
> +}
> +
>  static inline struct in6_ifaddr *
>  ifatoia6(struct ifaddr *ifa)
>  {