From: Alexander Bluhm <bluhm@openbsd.org>
Subject: raw IPv6 inp_cksum6 inp_icmp6filt
To: tech@openbsd.org
Date: Tue, 16 Apr 2024 19:46:00 +0200

Hi,

Document that both inp_cksum6 and inp_icmp6filt are protected by
net lock.  The inp_icmp6filt is ignored if the pointer is NULL, but
it is always set during attach.  Better allocate the ICMP6 filter
only when used in icmp6_ctloutput().

ok?

bluhm

Index: netinet/in_pcb.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
diff -u -p -r1.155 in_pcb.h
--- netinet/in_pcb.h	15 Apr 2024 18:31:04 -0000	1.155
+++ netinet/in_pcb.h	16 Apr 2024 14:40:25 -0000
@@ -175,9 +175,9 @@ struct inpcb {
 #define inp_ip6_minhlim inp_ip_minttl	/* minimum Hop Limit or drop */
 #define	inp_flowinfo	inp_hu.hu_ipv6.ip6_flow
 
-	int	inp_cksum6;
-	struct	icmp6_filter *inp_icmp6filt;
-	struct	pf_state_key *inp_pf_sk; /* [L] */
+	int	inp_cksum6;			/* [N] */
+	struct	icmp6_filter *inp_icmp6filt;	/* [N] */
+	struct	pf_state_key *inp_pf_sk;	/* [L] */
 	struct	mbuf *(*inp_upcall)(void *, struct mbuf *,
 		    struct ip *, struct ip6_hdr *, void *, int);
 	void	*inp_upcall_arg;
Index: netinet6/icmp6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
diff -u -p -r1.251 icmp6.c
--- netinet6/icmp6.c	3 Dec 2023 20:36:24 -0000	1.251
+++ netinet6/icmp6.c	16 Apr 2024 14:40:25 -0000
@@ -1702,24 +1702,20 @@ icmp6_ctloutput(int op, struct socket *s
 	case PRCO_SETOPT:
 		switch (optname) {
 		case ICMP6_FILTER:
-		    {
-			struct icmp6_filter *p;
-
-			if (m == NULL || m->m_len != sizeof(*p)) {
+			if (m == NULL ||
+			    m->m_len != sizeof(struct icmp6_filter)) {
 				error = EMSGSIZE;
 				break;
 			}
-			p = mtod(m, struct icmp6_filter *);
-			if (!p || !inp->inp_icmp6filt) {
-				error = EINVAL;
-				break;
+			if (inp->inp_icmp6filt == NULL) {
+				inp->inp_icmp6filt = malloc(
+				    sizeof(struct icmp6_filter), M_PCB,
+				    M_WAITOK);
 			}
-			bcopy(p, inp->inp_icmp6filt,
-				sizeof(struct icmp6_filter));
-			error = 0;
+			memcpy(inp->inp_icmp6filt,
+			    mtod(m, struct icmp6_filter *),
+			    sizeof(struct icmp6_filter));
 			break;
-		    }
-
 		default:
 			error = ENOPROTOOPT;
 			break;
@@ -1729,21 +1725,16 @@ icmp6_ctloutput(int op, struct socket *s
 	case PRCO_GETOPT:
 		switch (optname) {
 		case ICMP6_FILTER:
-		    {
-			struct icmp6_filter *p;
-
-			if (!inp->inp_icmp6filt) {
-				error = EINVAL;
+			m->m_len = sizeof(struct icmp6_filter);
+			if (inp->inp_icmp6filt == NULL) {
+				ICMP6_FILTER_SETPASSALL(
+				    mtod(m, struct icmp6_filter *));
 				break;
 			}
-			m->m_len = sizeof(struct icmp6_filter);
-			p = mtod(m, struct icmp6_filter *);
-			bcopy(inp->inp_icmp6filt, p,
-				sizeof(struct icmp6_filter));
-			error = 0;
+			memcpy(mtod(m, struct icmp6_filter *),
+			    inp->inp_icmp6filt,
+			    sizeof(struct icmp6_filter));
 			break;
-		    }
-
 		default:
 			error = ENOPROTOOPT;
 			break;
Index: netinet6/raw_ip6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/raw_ip6.c,v
diff -u -p -r1.183 raw_ip6.c
--- netinet6/raw_ip6.c	16 Apr 2024 12:40:40 -0000	1.183
+++ netinet6/raw_ip6.c	16 Apr 2024 14:40:25 -0000
@@ -207,7 +207,7 @@ rip6_input(struct mbuf **mp, int *offp, 
 		if (!IN6_IS_ADDR_UNSPECIFIED(&inp->inp_faddr6) &&
 		    !IN6_ARE_ADDR_EQUAL(&inp->inp_faddr6, &ip6->ip6_src))
 			continue;
-		if (proto == IPPROTO_ICMPV6 && inp->inp_icmp6filt) {
+		if (proto == IPPROTO_ICMPV6 && inp->inp_icmp6filt != NULL) {
 			if (ICMP6_FILTER_WILLBLOCK(type, inp->inp_icmp6filt))
 				continue;
 		}
@@ -613,13 +613,6 @@ rip6_attach(struct socket *so, int proto
 	inp->inp_ipv6.ip6_nxt = proto;
 	inp->inp_cksum6 = -1;
 
-	inp->inp_icmp6filt = malloc(sizeof(struct icmp6_filter), M_PCB,
-	    wait == M_WAIT ? M_WAITOK : M_NOWAIT);
-	if (inp->inp_icmp6filt == NULL) {
-		in_pcbdetach(inp);
-		return ENOMEM;
-	}
-	ICMP6_FILTER_SETPASSALL(inp->inp_icmp6filt);
 	return 0;
 }