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
