In article <20051114222417.GO12021@snowdrop.l8s.co.uk>,
David Laight  <david@l8s.co.uk> wrote:
>I was writing some code to send UDP RPC requests and discovered that 
>the code:
>	sock = socket(d->ai->ai_family, SOCK_DGRAM, IPPROTO_UDP);
>	range = IP_PORTRANGE_LOW;
>	setsockopt(sock, IPPROTO_IP, IP_PORTRANGE, &range, sizeof(range));
>	connect(sock, d->ai->ai_addr, d->ai->ai_addrlen);
>will transmit a UDP request with a source port of zero.
>(Whether executed by root or any other user.)
>
>Without the setsockopt() a high numbered port is used (as expected).
>
>Digging a little I discovered that in_pcbconnect() fails to pass the
>'proc' pointer to in_pcbbind() so that it cannot allocate a low-numbered
>port, and then fails to report the EACCESS error back to the user.
>It then blindly transmits the UDP datagram from an unbound port.
>
>Digging even further, it seems that when the IP_PORTRANGE_LOW stuff was
>added in rev 1.45 (Jan 1998 - from FreeBSD) no one noticed that the
>in_pcbbind() call in in_pcbconnect() could now fail (prior to that
>in_pcbbind() would only allocate ports above 1024).  Later someone
>made it return EAGAIN if all the local port were used - but still didn't
>worry about the EACCESS return.
>
>Before this ports below 1024 had to be explicitely bound - requiring
>the applications to loop through the port numbers.
>
>To behave sensibly the code needs to either:
>1) Always allocate a high port
>or
>2) Allocate a low port for root and a high port for non-root
>or
>3) Allocate a low port for root and fail EACCESS for non-root
>or
>4) Always fail EACCESS
>
>With the code above issing a bind() request is quite hard:
>	static char ad[64];
>	((struct sockaddr *)ad)->sa_family =
>		    ((struct sockaddr *)d->ai->ai_addr)->sa_family;
>	bind(sock, (void *)ad, d->ai->ai_addrlen);
>seems to be enough, but you are close to needing to know the format
>of the address buffer for the protocol.
>
>The patch below implements (3) above.
>
>	David
>
>Index: netinet/in_pcb.c
>===================================================================
>RCS file: /cvsroot/src/sys/netinet/in_pcb.c,v
>retrieving revision 1.100
>diff -u -p -r1.100 in_pcb.c
>--- netinet/in_pcb.c	29 May 2005 21:41:23 -0000	1.100
>+++ netinet/in_pcb.c	14 Nov 2005 21:59:38 -0000
>@@ -357,7 +357,7 @@ noname:
>  * then pick one.
>  */
> int
>-in_pcbconnect(void *v, struct mbuf *nam)
>+in_pcbconnect(void *v, struct mbuf *nam, struct proc *p)
> {
> 	struct inpcb *inp = v;
> 	struct in_ifaddr *ia = NULL;
>@@ -427,15 +427,14 @@ in_pcbconnect(void *v, struct mbuf *nam)
> 		return (EADDRINUSE);
> 	if (in_nullhost(inp->inp_laddr)) {
> 		if (inp->inp_lport == 0) {
>-			error = in_pcbbind(inp, (struct mbuf *)0,
>-			    (struct proc *)0);
>+			error = in_pcbbind(inp, NULL, p);
> 			/*
> 			 * This used to ignore the return value
> 			 * completely, but we need to check for
> 			 * ephemeral port shortage.
>-			 * XXX Should we check for other errors, too?
>+			 * And attempts to request low ports if not root.
> 			 */
>-			if (error == EAGAIN)
>+			if (error != 0)
> 				return (error);
> 		}
> 		inp->inp_laddr = ifaddr->sin_addr;
>Index: netinet/in_pcb.h
>===================================================================
>RCS file: /cvsroot/src/sys/netinet/in_pcb.h,v
>retrieving revision 1.39
>diff -u -p -r1.39 in_pcb.h
>--- netinet/in_pcb.h	12 Feb 2005 12:31:07 -0000	1.39
>+++ netinet/in_pcb.h	14 Nov 2005 21:59:38 -0000
>@@ -118,7 +118,7 @@ struct inpcb {
> void	in_losing(struct inpcb *);
> int	in_pcballoc(struct socket *, void *);
> int	in_pcbbind(void *, struct mbuf *, struct proc *);
>-int	in_pcbconnect(void *, struct mbuf *);
>+int	in_pcbconnect(void *, struct mbuf *, struct proc *);
> void	in_pcbdetach(void *);
> void	in_pcbdisconnect(void *);
> void	in_pcbinit(struct inpcbtable *, int, int);
>Index: netinet/tcp_input.c
>===================================================================
>RCS file: /cvsroot/src/sys/netinet/tcp_input.c,v
>retrieving revision 1.236
>diff -u -p -r1.236 tcp_input.c
>--- netinet/tcp_input.c	12 Aug 2005 14:41:00 -0000	1.236
>+++ netinet/tcp_input.c	14 Nov 2005 21:59:39 -0000
>@@ -3735,7 +3735,7 @@ syn_cache_get(struct sockaddr *src, stru
> 	am->m_len = src->sa_len;
> 	bcopy(src, mtod(am, caddr_t), src->sa_len);
> 	if (inp) {
>-		if (in_pcbconnect(inp, am)) {
>+		if (in_pcbconnect(inp, am, NULL)) {
> 			(void) m_free(am);
> 			goto resetandabort;
> 		}
>@@ -3756,7 +3756,7 @@ syn_cache_get(struct sockaddr *src, stru
> 				&sin6->sin6_addr.s6_addr32[3],
> 				sizeof(sin6->sin6_addr.s6_addr32[3]));
> 		}
>-		if (in6_pcbconnect(in6p, am)) {
>+		if (in6_pcbconnect(in6p, am, NULL)) {
> 			(void) m_free(am);
> 			goto resetandabort;
> 		}
>Index: netinet/tcp_usrreq.c
>===================================================================
>RCS file: /cvsroot/src/sys/netinet/tcp_usrreq.c,v
>retrieving revision 1.111
>diff -u -p -r1.111 tcp_usrreq.c
>--- netinet/tcp_usrreq.c	7 Sep 2005 17:58:13 -0000	1.111
>+++ netinet/tcp_usrreq.c	14 Nov 2005 21:59:39 -0000
>@@ -385,7 +385,7 @@ tcp_usrreq(struct socket *so, int req,
> 				if (error)
> 					break;
> 			}
>-			error = in_pcbconnect(inp, nam);
>+			error = in_pcbconnect(inp, nam, p);
> 		}
> #endif
> #ifdef INET6
>@@ -396,7 +396,7 @@ tcp_usrreq(struct socket *so, int req,
> 				if (error)
> 					break;
> 			}
>-			error = in6_pcbconnect(in6p, nam);
>+			error = in6_pcbconnect(in6p, nam, p);
> 			if (!error) {
> 				/* mapped addr case */
> 				if (IN6_IS_ADDR_V4MAPPED(&in6p->in6p_faddr))
>Index: netinet/udp_usrreq.c
>===================================================================
>RCS file: /cvsroot/src/sys/netinet/udp_usrreq.c,v
>retrieving revision 1.142
>diff -u -p -r1.142 udp_usrreq.c
>--- netinet/udp_usrreq.c	3 Sep 2005 18:01:07 -0000	1.142
>+++ netinet/udp_usrreq.c	14 Nov 2005 21:59:39 -0000
>@@ -1226,7 +1226,7 @@ udp_usrreq(struct socket *so, int req, s
> 		break;
> 
> 	case PRU_CONNECT:
>-		error = in_pcbconnect(inp, nam);
>+		error = in_pcbconnect(inp, nam, p);
> 		if (error)
> 			break;
> 		soisconnected(so);
>@@ -1268,7 +1268,7 @@ udp_usrreq(struct socket *so, int req, s
> 				error = EISCONN;
> 				goto die;
> 			}
>-			error = in_pcbconnect(inp, nam);
>+			error = in_pcbconnect(inp, nam, p);
> 			if (error)
> 				goto die;
> 		} else {
>Index: netinet6/in6_pcb.c
>===================================================================
>RCS file: /cvsroot/src/sys/netinet6/in6_pcb.c,v
>retrieving revision 1.67
>diff -u -p -r1.67 in6_pcb.c
>--- netinet6/in6_pcb.c	29 May 2005 21:43:51 -0000	1.67
>+++ netinet6/in6_pcb.c	14 Nov 2005 21:59:40 -0000
>@@ -344,9 +344,10 @@ in6_pcbbind(v, nam, p)
>  * then pick one.
>  */
> int
>-in6_pcbconnect(v, nam)
>+in6_pcbconnect(v, nam, p)
> 	void *v;
> 	struct mbuf *nam;
>+	struct proc *p;
> {
> 	struct in6pcb *in6p = v;
> 	struct in6_addr *in6a = NULL;
>@@ -449,8 +450,9 @@ in6_pcbconnect(v, nam)
> 	     in6p->in6p_laddr.s6_addr32[3] == 0))
> 	{
> 		if (in6p->in6p_lport == 0) {
>-			(void)in6_pcbbind(in6p, (struct mbuf *)0,
>-			    (struct proc *)0);
>+			error = in6_pcbbind(in6p, (struct mbuf *)0, p);
>+			if (error != 0)
>+				return error;
> 		}
> 		in6p->in6p_laddr = *in6a;
> 	}
>Index: netinet6/in6_pcb.h
>===================================================================
>RCS file: /cvsroot/src/sys/netinet6/in6_pcb.h,v
>retrieving revision 1.26
>diff -u -p -r1.26 in6_pcb.h
>--- netinet6/in6_pcb.h	29 May 2005 21:43:51 -0000	1.26
>+++ netinet6/in6_pcb.h	14 Nov 2005 21:59:40 -0000
>@@ -152,7 +152,7 @@ void	in6_losing __P((struct in6pcb *));
> void	in6_pcbinit __P((struct inpcbtable *, int, int));
> int	in6_pcballoc __P((struct socket *, void *));
> int	in6_pcbbind __P((void *, struct mbuf *, struct proc *));
>-int	in6_pcbconnect __P((void *, struct mbuf *));
>+int	in6_pcbconnect __P((void *, struct mbuf *, struct proc *));
> void	in6_pcbdetach __P((struct in6pcb *));
> void	in6_pcbdisconnect __P((struct in6pcb *));
> struct	in6pcb *in6_pcblookup_port __P((struct inpcbtable *, struct in6_addr *,
>Index: netinet6/udp6_usrreq.c
>===================================================================
>RCS file: /cvsroot/src/sys/netinet6/udp6_usrreq.c,v
>retrieving revision 1.70
>diff -u -p -r1.70 udp6_usrreq.c
>--- netinet6/udp6_usrreq.c	28 Aug 2005 21:01:02 -0000	1.70
>+++ netinet6/udp6_usrreq.c	14 Nov 2005 21:59:40 -0000
>@@ -336,7 +336,7 @@ udp6_usrreq(so, req, m, addr6, control, 
> 			break;
> 		}
> 		s = splsoftnet();
>-		error = in6_pcbconnect(in6p, addr6);
>+		error = in6_pcbconnect(in6p, addr6, p);
> 		splx(s);
> 		if (error == 0)
> 			soisconnected(so);

This looks fine to me.

christos