slirp: fix ICMP handling on macOS hosts
PATCH v3 1/1 slirp: fix ICMP handling on macOS hosts
On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when read from. On macOS, however, the socket acts like a SOCK_RAW socket and includes the IP header as well.
This change strips the extra IP header from the received packet on macOS before sending it to the guest. SOCK_DGRAM ICMP sockets aren't supported on other BSDs, but we enable this behavior for them as well to treat the sockets the same as raw sockets.
---
v2: check validity of inner_hlen and update len appropriately
v3: CONFIG_DARWIN -> CONFIG_BSD; add comment explaining #ifdef
slirp/ip_icmp.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
slirp/ip_icmp.c +23 -1
@@ -420,7 +420,29 @@ void icmp_receive(struct socket *so) icp = mtod(m, struct icmp *); id = icp->icmp_id; - len =qemu_recv (so->s, icp, m->m_len,0 ); + len =qemu_recv (so->s, icp,M_ROOM (m),0 ); +/* +* The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is inconsistent +* between host OSes. On Linux, only the ICMP header and payload is +* included. On macOS/Darwin, the socket acts like a raw socket and +* includes the IP header as well. On other BSDs, SOCK_DGRAM+IPPROTO_ICMP +* sockets aren't supported at all, so we treat them like raw sockets. It +* isn't possible to detect this difference at runtime, so we must use an +* #ifdef to determine if we need to remove the IP header. +*/ +#ifdef CONFIG_BSD +if (len >0 ) { + struct ip *inner_ip = mtod(m, struct ip *); +int inner_hlen = inner_ip->ip_hl <<2 ; +if (inner_hlen > len) { + len =-1 ; + errno = -EINVAL ; + }else { + len -= inner_hlen; +memmove (icp, (unsigned char *)icp + inner_hlen, len); + } + } +#endif icp->icmp_id = id; m->m_data -= hlen;
Signature · 1 line
2.18.0.865.gffc8e1a3cd6-goog
Samuel Thibault
AppliedAndrew Oates, le mar. 14 août 2018 22:35:21 -0400, a ecrit:
Quoted 8 lines
On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when read from. On macOS, however, the socket acts like a
SOCK_RAWsocket and includes the IP header as well.This change strips the extra IP header from the received packet on macOS before sending it to the guest.
SOCK_DGRAMICMP sockets aren't supported on other BSDs, but we enable this behavior for them as well to treat the sockets the same as raw sockets.
Applied to my tree, thanks!
Quoted 46 lines
Signed-off-by Andrew Oates <[email protected]>---
v2: check validity ofinner_hlenand update len appropriately
v3:CONFIG_DARWIN->CONFIG_BSD; add comment explaining #ifdefslirp/ip_icmp.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)slirp/ip_icmp.c +23
-1@@ -420,7 +420,29 @@ void icmp_receive(struct socket *so) icp = mtod(m, struct icmp *); id = icp->icmp_id; - len =qemu_recv (so->s, icp, m->m_len,0 ); + len =qemu_recv (so->s, icp,M_ROOM (m),0 ); +/* +* The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is inconsistent +* between host OSes. On Linux, only the ICMP header and payload is +* included. On macOS/Darwin, the socket acts like a raw socket and +* includes the IP header as well. On other BSDs, SOCK_DGRAM+IPPROTO_ICMP +* sockets aren't supported at all, so we treat them like raw sockets. It +* isn't possible to detect this difference at runtime, so we must use an +* #ifdef to determine if we need to remove the IP header. +*/ +#ifdef CONFIG_BSD +if (len >0 ) { + struct ip *inner_ip = mtod(m, struct ip *); +int inner_hlen = inner_ip->ip_hl <<2 ; +if (inner_hlen > len) { + len =-1 ; + errno = -EINVAL ; + }else { + len -= inner_hlen; +memmove (icp, (unsigned char *)icp + inner_hlen, len); + } + } +#endif icp->icmp_id = id; m->m_data -= hlen;Signature · 1 line
2.18.0.865.gffc8e1a3cd6-goog
Signature · 4 lines
Samuel As usual, this being a 1.3.x release, I haven't even compiled this kernel yet. So if it works, you should be doubly impressed. (Linus Torvalds, announcing kernel 1.3.3 on the linux-kernel mailing list.)
Quoted 9 lines · Andrew Oates
On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when read from. On macOS, however, the socket acts like a
SOCK_RAWsocket and includes the IP header as well.This change strips the extra IP header from the received packet on macOS before sending it to the guest.
SOCK_DGRAMICMP sockets aren't supported on other BSDs, but we enable this behavior for them as well to treat the sockets the same as raw sockets.
Quoted 5 lines
Signed-off-by Andrew Oates <[email protected]>---
v2: check validity ofinner_hlenand update len appropriately
v3:CONFIG_DARWIN->CONFIG_BSD; add comment explaining #ifdef
thanks
-- PMM
Andrew Oates, le mar. 14 août 2018 22:35:21 -0400, a ecrit:
Quoted 28 lines
On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when read from. On macOS, however, the socket acts like a
SOCK_RAWsocket and includes the IP header as well.This change strips the extra IP header from the received packet on macOS before sending it to the guest.
SOCK_DGRAMICMP sockets aren't supported on other BSDs, but we enable this behavior for them as well to treat the sockets the same as raw sockets.Signed-off-by Andrew Oates <[email protected]>---
v2: check validity ofinner_hlenand update len appropriately
v3:CONFIG_DARWIN->CONFIG_BSD; add comment explaining #ifdefslirp/ip_icmp.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)slirp/ip_icmp.c +2
-1@@ -420,7 +420,29 @@ void icmp_receive(struct socket *so) icp = mtod(m, struct icmp *); id = icp->icmp_id; - len =qemu_recv (so->s, icp, m->m_len,0 ); + len =qemu_recv (so->s, icp,M_ROOM (m),0 ); +/*
slirp/ip_icmp.c void icmp_receive(struct socket *so) · line 434
Quoted 10 lines
slirp/ip_icmp.c +10
-0+* The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is inconsistent +* between host OSes. On Linux, only the ICMP header and payload is +* included. On macOS/Darwin, the socket acts like a raw socket and +* includes the IP header as well. On other BSDs, SOCK_DGRAM+IPPROTO_ICMP +* sockets aren't supported at all, so we treat them like raw sockets. It +* isn't possible to detect this difference at runtime, so we must use an +* #ifdef to determine if we need to remove the IP header. +*/ +#ifdef CONFIG_BSD +if (len >0 ) {
Looking at it again, this used to be
if (len >= sizeof(struct ip)) {shouldn't be it that way so that
slirp/ip_icmp.c void icmp_receive(struct socket *so) · line 436
Quoted 2 lines
slirp/ip_icmp.c +2
-0+ struct ip *inner_ip = mtod(m, struct ip *); +int inner_hlen = inner_ip->ip_hl <<2 ;
Reading in the header doesn't give uninitialized values?
I guess that to be on the safe side and trigger explicit warnings, we should reject with EINVAL packets which are smaller than an IP header?
Quoted 15 lines
slirp/ip_icmp.c +9
-0+if (inner_hlen > len) { + len =-1 ; + errno = -EINVAL ; + }else { + len -= inner_hlen; +memmove (icp, (unsigned char *)icp + inner_hlen, len); + } + } +#endif icp->icmp_id = id; m->m_data -= hlen;Signature · 1 line
2.18.0.865.gffc8e1a3cd6-goog
Signature · 5 lines
Samuel <N> (* If you have a precise idea of the intended use of the following code, please <N> write to [email protected] and ask for the prize :-) <N> -- Eduardo (11/8/97) *) -+- N sur #ens-mim - et c'était un des développeurs -+-
Quoted 50 lines · Samuel Thibault
Andrew Oates, le mar. 14 août 2018 22:35:21 -0400, a ecrit:
Quoted 22 lines
On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when read from. On macOS, however, the socket acts like a
SOCK_RAWsocket and includes the IP header as well.This change strips the extra IP header from the received packet on macOS before sending it to the guest.
SOCK_DGRAMICMP sockets aren't supported on other BSDs, but we enable this behavior for them as well to treat the sockets the same as raw sockets.Signed-off-by Andrew Oates <[email protected]>---
v2: check validity ofinner_hlenand update len appropriately
v3:CONFIG_DARWIN->CONFIG_BSD; add comment explaining #ifdefslirp/ip_icmp.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)slirp/ip_icmp.c +0
-0slirp/ip_icmp.c void icmp_receive(struct socket *so) · line 425
Quoted 7 lines
slirp/ip_icmp.c +3
-1@@ -420,7 +420,29 @@ void icmp_receive(struct socket *so) icp = mtod(m, struct icmp *); id = icp->icmp_id; - len =qemu_recv (so->s, icp, m->m_len,0 ); + len =qemu_recv (so->s, icp,M_ROOM (m),0 ); +/* +* The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is inconsistent
slirp/ip_icmp.c void icmp_receive(struct socket *so)
Quoted 3 lines
slirp/ip_icmp.c +3
-0+* between host OSes. On Linux, only the ICMP header and payload is +* included. On macOS/Darwin, the socket acts like a raw socket and +* includes the IP header as well. On other BSDs, SOCK_DGRAM+IPPROTO_ICMP
slirp/ip_icmp.c void icmp_receive(struct socket *so) · line 429
Quoted 1 line
slirp/ip_icmp.c +1
-0+* sockets aren't supported at all, so we treat them like raw sockets. It
slirp/ip_icmp.c void icmp_receive(struct socket *so) · line 430
Quoted 1 line
slirp/ip_icmp.c +1
-0+* isn't possible to detect this difference at runtime, so we must use an
slirp/ip_icmp.c void icmp_receive(struct socket *so) · line 434
Quoted 4 lines
slirp/ip_icmp.c +4
-0+* #ifdef to determine if we need to remove the IP header. +*/ +#ifdef CONFIG_BSD +if (len >0 ) {Looking at it again, this used to be
if (len >= sizeof(struct ip)) {shouldn't be it that way so that
Quoted 8 lines
slirp/ip_icmp.c void icmp_receive(struct socket *so) · line 436
Quoted 2 lines
slirp/ip_icmp.c +2
-0+ struct ip *inner_ip = mtod(m, struct ip *); +int inner_hlen = inner_ip->ip_hl <<2 ;Reading in the header doesn't give uninitialized values?
I guess that to be on the safe side and trigger explicit warnings, we should reject with EINVAL packets which are smaller than an IP header?
Hmm, yeah, sorry. That got reverted in my patch and I didn't notice. Will re-send.
Quoted 25 lines
Quoted 14 lines
slirp/ip_icmp.c +9
-1+if (inner_hlen > len) { + len =-1 ; + errno = -EINVAL ; + }else { + len -= inner_hlen; +memmove (icp, (unsigned char *)icp + inner_hlen, len); + } + } +#endif icp->icmp_id = id; m->m_data -= hlen; --Quoted 2 lines
2.18.0.865.gffc8e1a3cd6-goog
--
Samuel
<N> (* If you have a precise idea of the intended use of the following code, please
<N> write to [email protected] and ask for the prize :-)
<N> -- Eduardo (11/8/97) *)
-+- N sur #ens-mim - et c'était un des développeurs -+-