On Mon, Aug 22, 2016 at 01:21:47PM +0200, Martin Pieuchot wrote: > When it comes to reference counting in the receiving path, route entries > act as proxy for interface addresses. In other words you CANNOT > dereference ``rt->rt_ifa'' after calling rtfree(9). > > Diff below fixes that in icmp_reflect(), ok?
Good catch. OK claudio@ > Index: netmpls/mpls_input.c > =================================================================== > RCS file: /cvs/src/sys/netmpls/mpls_input.c,v > retrieving revision 1.56 > diff -u -p -r1.56 mpls_input.c > --- netmpls/mpls_input.c 11 Jul 2016 09:23:06 -0000 1.56 > +++ netmpls/mpls_input.c 22 Aug 2016 11:20:48 -0000 > @@ -385,8 +385,9 @@ mpls_do_error(struct mbuf *m, int type, > m_freem(m); > return (NULL); > } > - rtfree(rt); > + /* It is safe to dereference ``ia'' iff ``rt'' is valid. */ > error = icmp_reflect(m, NULL, ia); > + rtfree(rt); > if (error) > return (NULL); > > Index: netinet/ip_icmp.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_icmp.c,v > retrieving revision 1.151 > diff -u -p -r1.151 ip_icmp.c > --- netinet/ip_icmp.c 9 Dec 2015 09:27:40 -0000 1.151 > +++ netinet/ip_icmp.c 22 Aug 2016 11:21:02 -0000 > @@ -702,7 +702,7 @@ icmp_reflect(struct mbuf *m, struct mbuf > struct ip *ip = mtod(m, struct ip *); > struct mbuf *opts = NULL; > struct sockaddr_in sin; > - struct rtentry *rt; > + struct rtentry *rt = NULL; > int optlen = (ip->ip_hl << 2) - sizeof(struct ip); > u_int rtableid; > > @@ -733,7 +733,6 @@ icmp_reflect(struct mbuf *m, struct mbuf > if (rtisvalid(rt) && > ISSET(rt->rt_flags, RTF_LOCAL|RTF_BROADCAST)) > ia = ifatoia(rt->rt_ifa); > - rtfree(rt); > } > > /* > @@ -742,6 +741,8 @@ icmp_reflect(struct mbuf *m, struct mbuf > * drop the packet as there is no path to the host. > */ > if (ia == NULL) { > + rtfree(rt); > + > memset(&sin, 0, sizeof(sin)); > sin.sin_len = sizeof(sin); > sin.sin_family = AF_INET; > @@ -756,12 +757,14 @@ icmp_reflect(struct mbuf *m, struct mbuf > } > > ia = ifatoia(rt->rt_ifa); > - rtfree(rt); > } > > ip->ip_dst = ip->ip_src; > - ip->ip_src = ia->ia_addr.sin_addr; > ip->ip_ttl = MAXTTL; > + > + /* It is safe to dereference ``ia'' iff ``rt'' is valid. */ > + ip->ip_src = ia->ia_addr.sin_addr; > + rtfree(rt); > > if (optlen > 0) { > u_char *cp; > -- :wq Claudio