On Mon, Mar 21, 2022 at 02:17:21PM +1000, David Gwynne wrote:
> in_pcbselsrc has this:
>
> ifp = if_get(mopts->imo_ifidx);
> if (ifp != NULL) {
> if (ifp->if_rdomain == rtable_l2(rtableid))
> IFP_TO_IA(ifp, ia);
> if (ia == NULL) {
> if_put(ifp);
> return (EADDRNOTAVAIL);
> }
>
> *insrc = ia->ia_addr.sin_addr;
> if_put(ifp);
> return (0);
> }
>
> which looks very much like it releases a reference to the interface
> holding the address it's passing back to the caller to use.
This seems indeed to be an issue.
> this diff has it copy the address to memory the caller provides instead.
>
> ok?
I think it makes to code overall a bit simpler.
OK claudio@
In in_pcbselsrc() you could even eliminate laddr and just replace it with
inp->inp_laddr. Or assign to laddr instead of making it a pointer.
> Index: in_pcb.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.262
> diff -u -p -r1.262 in_pcb.c
> --- in_pcb.c 21 Mar 2022 03:51:09 -0000 1.262
> +++ in_pcb.c 21 Mar 2022 04:10:24 -0000
> @@ -476,7 +476,7 @@ in_pcbpickport(u_int16_t *lport, void *l
> int
> in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
> {
> - struct in_addr *ina = NULL;
> + struct in_addr ina;
> struct sockaddr_in *sin;
> int error;
>
> @@ -495,7 +495,7 @@ in_pcbconnect(struct inpcb *inp, struct
> 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);
> @@ -506,13 +506,13 @@ in_pcbconnect(struct inpcb *inp, struct
> 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;
> @@ -870,7 +870,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;
> @@ -886,9 +886,9 @@ in_pcbselsrc(struct in_addr **insrc, str
> * 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);
> }
>
> @@ -911,7 +911,7 @@ in_pcbselsrc(struct in_addr **insrc, str
> return (EADDRNOTAVAIL);
> }
>
> - *insrc = &ia->ia_addr.sin_addr;
> + *insrc = ia->ia_addr.sin_addr;
> if_put(ifp);
> return (0);
> }
> @@ -962,7 +962,7 @@ in_pcbselsrc(struct in_addr **insrc, str
> 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);
> }
> }
> @@ -971,7 +971,7 @@ in_pcbselsrc(struct in_addr **insrc, str
> if (ia == NULL)
> return (EADDRNOTAVAIL);
>
> - *insrc = &ia->ia_addr.sin_addr;
> + *insrc = ia->ia_addr.sin_addr;
> return (0);
> }
>
> Index: in_pcb.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_pcb.h,v
> retrieving revision 1.125
> diff -u -p -r1.125 in_pcb.h
> --- in_pcb.h 14 Mar 2022 22:38:43 -0000 1.125
> +++ in_pcb.h 21 Mar 2022 04:10:25 -0000
> @@ -305,7 +305,7 @@ void in_setpeeraddr(struct inpcb *, str
> 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 *);
>
> Index: udp_usrreq.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
> retrieving revision 1.274
> diff -u -p -r1.274 udp_usrreq.c
> --- udp_usrreq.c 14 Mar 2022 22:38:43 -0000 1.274
> +++ udp_usrreq.c 21 Mar 2022 04:10:25 -0000
> @@ -857,7 +857,7 @@ udp_output(struct inpcb *inp, struct mbu
> 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
> @@ -957,14 +957,14 @@ udp_output(struct inpcb *inp, struct mbu
> (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;
> }
>
> /*
> @@ -985,7 +985,7 @@ udp_output(struct inpcb *inp, struct mbu
> 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;
> Index: raw_ip.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/raw_ip.c,v
> retrieving revision 1.124
> diff -u -p -r1.124 raw_ip.c
> --- raw_ip.c 21 Mar 2022 04:00:56 -0000 1.124
> +++ raw_ip.c 21 Mar 2022 04:10:25 -0000
> @@ -271,13 +271,9 @@ rip_output(struct mbuf *m, struct socket
> }
>
> 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
>
--
:wq Claudio
