have in_pcbselsrc copy the selected address to memory provided by the caller.

having it return a pointer to something that has a lifetime managed
by a lock without accounting for it or taking a reference count or
anything like that is asking for trouble. copying the address to
caller provded memory while still inside the lock is a lot safer.

discussed with visa@
ok bluhm@ claudio@

diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
index 9d4749f..a725b17 100644 (file)
--- a/sys/netinet/in_pcb.c
+++ b/sys/netinet/in_pcb.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: in_pcb.c,v 1.265 2022/04/14 14:10:22 claudio Exp $    */
+/*     $OpenBSD: in_pcb.c,v 1.266 2022/05/15 09:12:20 dlg Exp $        */
 /*     $NetBSD: in_pcb.c,v 1.25 1996/02/13 23:41:53 christos Exp $     */
 
 /*
@@ -480,7 +480,7 @@ in_pcbpickport(u_int16_t *lport, void *laddr, int wild, struct inpcb *inp,
 int
 in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
 {
-       struct in_addr *ina = NULL;
+       struct in_addr ina;
        struct sockaddr_in *sin;
        int error;
 
@@ -499,7 +499,7 @@ in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
                return (error);
 
        if (in_pcbhashlookup(inp->inp_table, sin->sin_addr, sin->sin_port,
-           *ina, inp->inp_lport, inp->inp_rtableid) != NULL)
+           ina, inp->inp_lport, inp->inp_rtableid) != NULL)
                return (EADDRINUSE);
 
        KASSERT(inp->inp_laddr.s_addr == INADDR_ANY || inp->inp_lport);
@@ -510,13 +510,13 @@ in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
                        if (error)
                                return (error);
                        if (in_pcbhashlookup(inp->inp_table, sin->sin_addr,
-                           sin->sin_port, *ina, inp->inp_lport,
+                           sin->sin_port, ina, inp->inp_lport,
                            inp->inp_rtableid) != NULL) {
                                inp->inp_lport = 0;
                                return (EADDRINUSE);
                        }
                }
-               inp->inp_laddr = *ina;
+               inp->inp_laddr = ina;
        }
        inp->inp_faddr = sin->sin_addr;
        inp->inp_fport = sin->sin_port;
@@ -893,7 +893,7 @@ in_pcbrtentry(struct inpcb *inp)
  * an entry to the caller for later use.
  */
 int
-in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
+in_pcbselsrc(struct in_addr *insrc, struct sockaddr_in *sin,
     struct inpcb *inp)
 {
        struct ip_moptions *mopts = inp->inp_moptions;
@@ -909,9 +909,9 @@ in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
         * If the socket(if any) is already bound, use that bound address
         * unless it is INADDR_ANY or INADDR_BROADCAST.
         */
-       if (laddr && laddr->s_addr != INADDR_ANY &&
+       if (laddr->s_addr != INADDR_ANY &&
            laddr->s_addr != INADDR_BROADCAST) {
-               *insrc = laddr;
+               *insrc = *laddr;
                return (0);
        }
 
@@ -934,7 +934,7 @@ in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
                                return (EADDRNOTAVAIL);
                        }
 
-                       *insrc = &ia->ia_addr.sin_addr;
+                       *insrc = ia->ia_addr.sin_addr;
                        if_put(ifp);
                        return (0);
                }
@@ -985,7 +985,7 @@ in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
                        struct ifaddr *ifa;
                        if ((ifa = ifa_ifwithaddr(ip4_source, rtableid)) !=
                            NULL && ISSET(ifa->ifa_ifp->if_flags, IFF_UP)) {
-                               *insrc = &satosin(ip4_source)->sin_addr;
+                               *insrc = satosin(ip4_source)->sin_addr;
                                return (0);
                        }
                }
@@ -994,7 +994,7 @@ in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
        if (ia == NULL)
                return (EADDRNOTAVAIL);
 
-       *insrc = &ia->ia_addr.sin_addr;
+       *insrc = ia->ia_addr.sin_addr;
        return (0);
 }
 

diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h
index 36ad80d..c14aa4b 100644 (file)
--- a/sys/netinet/in_pcb.h
+++ b/sys/netinet/in_pcb.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: in_pcb.h,v 1.128 2022/03/21 23:37:09 bluhm Exp $      */
+/*     $OpenBSD: in_pcb.h,v 1.129 2022/05/15 09:12:20 dlg Exp $        */
 /*     $NetBSD: in_pcb.h,v 1.14 1996/02/13 23:42:00 christos Exp $     */
 
 /*
@@ -315,7 +315,7 @@ void         in_setpeeraddr(struct inpcb *, struct mbuf *);
 void    in_setsockaddr(struct inpcb *, struct mbuf *);
 int     in_baddynamic(u_int16_t, u_int16_t);
 int     in_rootonly(u_int16_t, u_int16_t);
-int     in_pcbselsrc(struct in_addr **, struct sockaddr_in *, struct inpcb *);
+int     in_pcbselsrc(struct in_addr *, struct sockaddr_in *, struct inpcb *);
 struct rtentry *
        in_pcbrtentry(struct inpcb *);
 

diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c
index bb43a7e..7e8a062 100644 (file)
--- a/sys/netinet/raw_ip.c
+++ b/sys/netinet/raw_ip.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: raw_ip.c,v 1.127 2022/03/23 17:22:28 bluhm Exp $      */
+/*     $OpenBSD: raw_ip.c,v 1.128 2022/05/15 09:12:20 dlg Exp $        */
 /*     $NetBSD: raw_ip.c,v 1.25 1996/02/18 18:58:33 christos Exp $     */
 
 /*
@@ -276,13 +276,9 @@ rip_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
        }
 
        if (ip->ip_src.s_addr == INADDR_ANY) {
-               struct in_addr *laddr;
-
-               error = in_pcbselsrc(&laddr, dst, inp);
+               error = in_pcbselsrc(&ip->ip_src, dst, inp);
                if (error != 0)
                        return (error);
-
-               ip->ip_src = *laddr;
        }
 
 #ifdef INET6

diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c
index a8dc168..84592f9 100644 (file)
--- a/sys/netinet/udp_usrreq.c
+++ b/sys/netinet/udp_usrreq.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: udp_usrreq.c,v 1.277 2022/03/21 23:37:09 bluhm Exp $  */
+/*     $OpenBSD: udp_usrreq.c,v 1.278 2022/05/15 09:12:20 dlg Exp $    */
 /*     $NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $ */
 
 /*
@@ -866,7 +866,7 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct mbuf *addr,
        u_int32_t ipsecflowinfo = 0;
        struct sockaddr_in src_sin;
        int len = m->m_pkthdr.len;
-       struct in_addr *laddr;
+       struct in_addr laddr;
        int error = 0;
 
 #ifdef DIAGNOSTIC
@@ -966,14 +966,14 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct mbuf *addr,
                            (error =
                            in_pcbaddrisavail(inp, &src_sin, 0, curproc)))
                                goto release;
-                       laddr = &src_sin.sin_addr;
+                       laddr = src_sin.sin_addr;
                }
        } else {
                if (inp->inp_faddr.s_addr == INADDR_ANY) {
                        error = ENOTCONN;
                        goto release;
                }
-               laddr = &inp->inp_laddr;
+               laddr = inp->inp_laddr;
        }
 
        /*
@@ -994,7 +994,7 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct mbuf *addr,
        bzero(ui->ui_x1, sizeof ui->ui_x1);
        ui->ui_pr = IPPROTO_UDP;
        ui->ui_len = htons((u_int16_t)len + sizeof (struct udphdr));
-       ui->ui_src = *laddr;
+       ui->ui_src = laddr;
        ui->ui_dst = sin ? sin->sin_addr : inp->inp_faddr;
        ui->ui_sport = inp->inp_lport;
        ui->ui_dport = sin ? sin->sin_port : inp->inp_fport;