From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48512) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTEGk-0002NO-P8 for qemu-devel@nongnu.org; Tue, 09 Feb 2016 14:48:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTEGh-0001sA-C8 for qemu-devel@nongnu.org; Tue, 09 Feb 2016 14:48:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43654) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTEGh-0001rx-2n for qemu-devel@nongnu.org; Tue, 09 Feb 2016 14:48:11 -0500 References: From: Thomas Huth Message-ID: <56BA4272.60101@redhat.com> Date: Tue, 9 Feb 2016 20:48:02 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCHv7 2/9] slirp: Adding ICMPv6 error sending List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Thibault , qemu-devel@nongnu.org Cc: zhanghailiang , Li Zhijian , Stefan Hajnoczi , Jason Wang , Dave Gilbert , Vasiliy Tolstov , Huangpeng , Yann Bordenave , Gonglei , Jan Kiszka , Guillaume Subiron On 08.02.2016 11:28, Samuel Thibault wrote: > From: Yann Bordenave >=20 > Disambiguation : icmp_error is renamed into icmp_send_error, since it > doesn't manage errors, but only sends ICMP Error messages. You could maybe also put the icmp_error (for IPv4) related stuff into a separate patch ... but I don't mind too much, this patch is also still readable as it is right now. > Adding icmp6_send_error to send ICMPv6 Error messages. This function is > simpler than the v4 version. > Adding some calls in various functions to send ICMP errors, when a > received packet is too big, or when its hop limit is 0. >=20 > Signed-off-by: Yann Bordenave > Signed-off-by: Samuel Thibault > --- > slirp/ip6_icmp.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++= ++++++++ > slirp/ip6_icmp.h | 10 ++++++++++ > slirp/ip6_input.c | 15 ++++++++------ > slirp/ip_icmp.c | 12 +++++------ > slirp/ip_icmp.h | 4 ++-- > slirp/ip_input.c | 8 ++++---- > slirp/socket.c | 4 ++-- > slirp/tcp_input.c | 2 +- > slirp/udp.c | 3 ++- > 9 files changed, 96 insertions(+), 22 deletions(-) >=20 > diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c > index 13f89af..41e034b 100644 > --- a/slirp/ip6_icmp.c > +++ b/slirp/ip6_icmp.c > @@ -67,6 +67,66 @@ static void icmp6_send_echoreply(struct mbuf *m, Sli= rp *slirp, struct ip6 *ip, > ip6_output(NULL, t, 0); > } > =20 > +void icmp6_send_error(struct mbuf *m, uint8_t type, uint8_t code) > +{ > + Slirp *slirp =3D m->slirp; > + struct mbuf *t =3D m_get(slirp); > + struct ip6 *ip =3D mtod(m, struct ip6 *); > + > + char addrstr[INET6_ADDRSTRLEN]; > + DEBUG_CALL("icmp_send_error"); > + DEBUG_ARGS((dfd, " type =3D %d, code =3D %d\n", type, code)); > + > + /* IPv6 packet */ > + struct ip6 *rip =3D mtod(t, struct ip6 *); > + rip->ip_src =3D (struct in6_addr)LINKLOCAL_ADDR; > + if (in6_multicast(ip->ip_src) || in6_unspecified(ip->ip_src)) { > + /* TODO icmp error? */ > + return; I think you're leaking the buffer from "struct mbuf *t =3D m_get(slirp)" here ... so "t" should be assigned after the above check instead. > + } > + rip->ip_dst =3D ip->ip_src; > + inet_ntop(AF_INET6, &rip->ip_dst, addrstr, INET6_ADDRSTRLEN); > + DEBUG_ARG("target =3D %s", addrstr); > + > + rip->ip_nh =3D IPPROTO_ICMPV6; > + const int error_data_len =3D min(m->m_len, > + IF_MTU - (sizeof(struct ip6) + ICMP6_ERROR_MINLEN)); > + rip->ip_pl =3D htons(ICMP6_ERROR_MINLEN + error_data_len); > + t->m_len =3D sizeof(struct ip6) + ntohs(rip->ip_pl); > + > + /* ICMPv6 packet */ > + t->m_data +=3D sizeof(struct ip6); > + struct icmp6 *ricmp =3D mtod(t, struct icmp6 *); > + ricmp->icmp6_type =3D type; > + ricmp->icmp6_code =3D code; > + ricmp->icmp6_cksum =3D 0; > + > + switch (type) { > + case ICMP6_UNREACH: > + case ICMP6_TIMXCEED: > + ricmp->icmp6_err.unused =3D 0; > + break; > + case ICMP6_TOOBIG: > + ricmp->icmp6_err.mtu =3D htonl(IF_MTU); > + break; > + case ICMP6_PARAMPROB: > + /* TODO: Handle this case */ > + break; > + default: > + g_assert_not_reached(); > + break; > + } > + t->m_data +=3D ICMP6_ERROR_MINLEN; > + memcpy(t->m_data, m->m_data, error_data_len); > + > + /* Checksum */ > + t->m_data -=3D ICMP6_ERROR_MINLEN; > + t->m_data -=3D sizeof(struct ip6); > + ricmp->icmp6_cksum =3D ip6_cksum(t); > + > + ip6_output(NULL, t, 0); > +} > + > /* > * Process a NDP message > */ > diff --git a/slirp/ip6_icmp.h b/slirp/ip6_icmp.h > index 1ae003b..f47c29f 100644 > --- a/slirp/ip6_icmp.h > +++ b/slirp/ip6_icmp.h > @@ -22,6 +22,12 @@ struct icmp6_echo { /* Echo Messages */ > uint16_t seq_num; > }; > =20 > +union icmp6_error_body { > + uint32_t unused; > + uint32_t pointer; > + uint32_t mtu; > +}; > + > /* > * NDP Messages > */ > @@ -85,6 +91,7 @@ struct icmp6 { > uint8_t icmp6_code; /* type sub code */ > uint16_t icmp6_cksum; /* ones complement cksum of struct= */ > union { > + union icmp6_error_body error_body; > struct icmp6_echo echo; > struct ndp_rs ndp_rs; > struct ndp_ra ndp_ra; > @@ -92,6 +99,7 @@ struct icmp6 { > struct ndp_na ndp_na; > struct ndp_redirect ndp_redirect; > } icmp6_body; > +#define icmp6_err icmp6_body.error_body > #define icmp6_echo icmp6_body.echo > #define icmp6_nrs icmp6_body.ndp_rs > #define icmp6_nra icmp6_body.ndp_ra > @@ -101,6 +109,7 @@ struct icmp6 { > } QEMU_PACKED; > =20 > #define ICMP6_MINLEN 4 > +#define ICMP6_ERROR_MINLEN 8 > #define ICMP6_ECHO_MINLEN 8 > #define ICMP6_NDP_RS_MINLEN 8 > #define ICMP6_NDP_RA_MINLEN 16 > @@ -238,6 +247,7 @@ struct ndpopt { > void icmp6_init(Slirp *slirp); > void icmp6_cleanup(Slirp *slirp); > void icmp6_input(struct mbuf *); > +void icmp6_send_error(struct mbuf *m, uint8_t type, uint8_t code); > void ndp_send_ra(Slirp *slirp); > void ndp_send_ns(Slirp *slirp, struct in6_addr addr); > =20 > diff --git a/slirp/ip6_input.c b/slirp/ip6_input.c > index e1e6229..828f47c 100644 > --- a/slirp/ip6_input.c > +++ b/slirp/ip6_input.c > @@ -33,7 +33,7 @@ void ip6_input(struct mbuf *m) > DEBUG_ARG("m_len =3D %d", m->m_len); > =20 > if (m->m_len < sizeof(struct ip6)) { > - return; > + goto bad; > } I think you could also merge this hunk into the previous patch instead. > ip6 =3D mtod(m, struct ip6 *); > @@ -42,9 +42,14 @@ void ip6_input(struct mbuf *m) > goto bad; > } > =20 > + if (ntohs(ip6->ip_pl) > IF_MTU) { > + icmp6_send_error(m, ICMP6_TOOBIG, 0); > + goto bad; > + } > + > /* check ip_ttl for a correct ICMP reply */ > if (ip6->ip_hl =3D=3D 0) { > - /*icmp_error(m, ICMP_TIMXCEED,ICMP_TIMXCEED_INTRANS, 0,"ttl");= */ > + icmp6_send_error(m, ICMP6_TIMXCEED, ICMP6_TIMXCEED_INTRANS); > goto bad; > } > =20 > @@ -52,14 +57,12 @@ void ip6_input(struct mbuf *m) > * Switch out to protocol's input routine. > */ > switch (ip6->ip_nh) { > -#if 0 > case IPPROTO_TCP: > - tcp_input(m, hlen, (struct socket *)NULL); > + icmp6_send_error(m, ICMP6_UNREACH, ICMP6_UNREACH_NO_ROUTE); > break; > case IPPROTO_UDP: > - udp_input(m, hlen); > + icmp6_send_error(m, ICMP6_UNREACH, ICMP6_UNREACH_NO_ROUTE); > break; > -#endif > case IPPROTO_ICMPV6: > icmp6_input(m); > break; > diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c > index ace3982..590dada 100644 > --- a/slirp/ip_icmp.c > +++ b/slirp/ip_icmp.c > @@ -38,7 +38,7 @@ > /* Be nice and tell them it's just a pseudo-ping packet */ > static const char icmp_ping_msg[] =3D "This is a pseudo-PING packet us= ed by Slirp to emulate ICMP ECHO-REQUEST packets.\n"; > =20 > -/* list of actions for icmp_error() on RX of an icmp message */ > +/* list of actions for icmp_send_error() on RX of an icmp message */ > static const int icmp_flush[19] =3D { > /* ECHO REPLY (0) */ 0, > 1, > @@ -101,7 +101,7 @@ static int icmp_send(struct socket *so, struct mbuf= *m, int hlen) > (struct sockaddr *)&addr, sizeof(addr)) =3D=3D -1) { > DEBUG_MISC((dfd, "icmp_input icmp sendto tx errno =3D %d-%s\n"= , > errno, strerror(errno))); > - icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_NET, 0, strerror(errn= o)); > + icmp_send_error(m, ICMP_UNREACH, ICMP_UNREACH_NET, 0, strerror= (errno)); > icmp_detach(so); > } > =20 > @@ -189,7 +189,7 @@ icmp_input(struct mbuf *m, int hlen) > (struct sockaddr *)&addr, sizeof(addr)) =3D=3D -1) { > DEBUG_MISC((dfd,"icmp_input udp sendto tx errno =3D %d-%s\n", > errno,strerror(errno))); > - icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errno)); > + icmp_send_error(m, ICMP_UNREACH, ICMP_UNREACH_NET, 0, strerror(errno)= ); > udp_detach(so); > } > } /* if ip->ip_dst.s_addr =3D=3D alias_addr.s_addr */ > @@ -235,7 +235,7 @@ end_error: > =20 > #define ICMP_MAXDATALEN (IP_MSS-28) > void > -icmp_error(struct mbuf *msrc, u_char type, u_char code, int minsize, > +icmp_send_error(struct mbuf *msrc, u_char type, u_char code, int minsi= ze, > const char *message) > { > unsigned hlen, shlen, s_ip_len; > @@ -243,7 +243,7 @@ icmp_error(struct mbuf *msrc, u_char type, u_char c= ode, int minsize, > register struct icmp *icp; > register struct mbuf *m; > =20 > - DEBUG_CALL("icmp_error"); > + DEBUG_CALL("icmp_send_error"); > DEBUG_ARG("msrc =3D %p", msrc); > DEBUG_ARG("msrc_len =3D %d", msrc->m_len); > =20 > @@ -433,7 +433,7 @@ void icmp_receive(struct socket *so) > } > DEBUG_MISC((dfd, " udp icmp rx errno =3D %d-%s\n", errno, > strerror(errno))); > - icmp_error(so->so_m, ICMP_UNREACH, error_code, 0, strerror(err= no)); > + icmp_send_error(so->so_m, ICMP_UNREACH, error_code, 0, strerro= r(errno)); > } else { > icmp_reflect(so->so_m); > so->so_m =3D NULL; /* Don't m_free() it again! */ > diff --git a/slirp/ip_icmp.h b/slirp/ip_icmp.h > index be4426b..846761d 100644 > --- a/slirp/ip_icmp.h > +++ b/slirp/ip_icmp.h > @@ -156,8 +156,8 @@ struct icmp { > void icmp_init(Slirp *slirp); > void icmp_cleanup(Slirp *slirp); > void icmp_input(struct mbuf *, int); > -void icmp_error(struct mbuf *msrc, u_char type, u_char code, int minsi= ze, > - const char *message); > +void icmp_send_error(struct mbuf *msrc, u_char type, u_char code, int = minsize, > + const char *message); > void icmp_reflect(struct mbuf *); > void icmp_receive(struct socket *so); > void icmp_detach(struct socket *so); > diff --git a/slirp/ip_input.c b/slirp/ip_input.c > index e4855ae..16fb2cb 100644 > --- a/slirp/ip_input.c > +++ b/slirp/ip_input.c > @@ -132,9 +132,9 @@ ip_input(struct mbuf *m) > m_adj(m, ip->ip_len - m->m_len); > =20 > /* check ip_ttl for a correct ICMP reply */ > - if(ip->ip_ttl=3D=3D0) { > - icmp_error(m, ICMP_TIMXCEED,ICMP_TIMXCEED_INTRANS, 0,"ttl"); > - goto bad; > + if (ip->ip_ttl =3D=3D 0) { > + icmp_send_error(m, ICMP_TIMXCEED, ICMP_TIMXCEED_INTRANS, 0, "ttl"= ); > + goto bad; > } > =20 > /* > @@ -637,7 +637,7 @@ typedef uint32_t n_time; > } > return (0); > bad: > - icmp_error(m, type, code, 0, 0); > + icmp_send_error(m, type, code, 0, 0); > =20 > return (1); > } > diff --git a/slirp/socket.c b/slirp/socket.c > index 2b5453e..32b1ba3 100644 > --- a/slirp/socket.c > +++ b/slirp/socket.c > @@ -463,7 +463,7 @@ sorecvfrom(struct socket *so) > =20 > DEBUG_MISC((dfd," udp icmp rx errno =3D %d-%s\n", > errno,strerror(errno))); > - icmp_error(so->so_m, ICMP_UNREACH,code, 0,strerror(errno)); > + icmp_send_error(so->so_m, ICMP_UNREACH, code, 0, strerror(errno))= ; > } else { > icmp_reflect(so->so_m); > so->so_m =3D NULL; /* Don't m_free() it again! */ > @@ -511,7 +511,7 @@ sorecvfrom(struct socket *so) > else if(errno =3D=3D ENETUNREACH) code=3DICMP_UNREACH_NET; > =20 > DEBUG_MISC((dfd," rx error, tx icmp ICMP_UNREACH:%i\n", code)); > - icmp_error(so->so_m, ICMP_UNREACH,code, 0,strerror(errno)); > + icmp_send_error(so->so_m, ICMP_UNREACH, code, 0, strerror(errno))= ; > m_free(m); > } else { > /* > diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c > index 2027a75..5f845da 100644 > --- a/slirp/tcp_input.c > +++ b/slirp/tcp_input.c > @@ -608,7 +608,7 @@ findso: > m->m_data -=3D sizeof(struct tcpiphdr)+off-sizeof(struct tcphdr= ); > m->m_len +=3D sizeof(struct tcpiphdr)+off-sizeof(struct tcphdr= ); > *ip=3Dsave_ip; > - icmp_error(m, ICMP_UNREACH,code, 0,strerror(errno)); > + icmp_send_error(m, ICMP_UNREACH, code, 0, strerror(errno)); > } > tcp_close(tp); > m_free(m); > diff --git a/slirp/udp.c b/slirp/udp.c > index 6b39cab..be012fb 100644 > --- a/slirp/udp.c > +++ b/slirp/udp.c > @@ -209,7 +209,8 @@ udp_input(register struct mbuf *m, int iphlen) > m->m_data -=3D iphlen; > *ip=3Dsave_ip; > DEBUG_MISC((dfd,"udp tx errno =3D %d-%s\n",errno,strerror(errno))); > - icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errno)); > + icmp_send_error(m, ICMP_UNREACH, ICMP_UNREACH_NET, 0, > + strerror(errno)); > goto bad; > } > =20 >=20