On Mon, Mar 21, 2022 at 11:13:24AM +0100, Claudio Jeker wrote:
> 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.
I would replace laddr with inp->inp_laddr.
OK bluhm@
> > 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
