ASAN detects an "unknown-crash" when running pxe-test:
/ppc64/pxe/spapr-vlan: =================================================================
==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at pc 0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0
READ of size 128 at 0x7f6dcd298d30 thread T2
#0 0x55e22218830c in tftp_session_allocate /home/elmarco/src/qq/slirp/tftp.c:73
#1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289
#2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446
#3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82
#4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67
Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame
#0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13
This frame has 3 object(s):
[32, 48) '<unknown>'
[96, 124) 'lhost' <== Memory access at offset 96 partially overflows this variable
[160, 200) 'save_ip' <== Memory access at offset 96 partially underflows this variable
The sockaddr_storage pointer is the sockaddr_in6 lhost on the
stack. Copy only the source addr size.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
slirp/tftp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/slirp/tftp.c b/slirp/tftp.c
index 50e714807d..a9bc4bb1b6 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct sockaddr_storage *srcsas,
found:
memset(spt, 0, sizeof(*spt));
- spt->client_addr = *srcsas;
+ memcpy(&spt->client_addr, srcsas, sockaddr_size(srcsas));
spt->fd = -1;
spt->block_size = 512;
spt->client_port = tp->udp.uh_sport;
--
2.12.0.191.gc5d8de91d
Marc-André Lureau, on jeu. 23 mars 2017 15:31:56 +0400, wrote: > ASAN detects an "unknown-crash" when running pxe-test: > > /ppc64/pxe/spapr-vlan: ================================================================= > ==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at pc 0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0 > READ of size 128 at 0x7f6dcd298d30 thread T2 > #0 0x55e22218830c in tftp_session_allocate /home/elmarco/src/qq/slirp/tftp.c:73 > #1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289 > #2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446 > #3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82 > #4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67 > > Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame > #0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13 > > This frame has 3 object(s): > [32, 48) '<unknown>' > [96, 124) 'lhost' <== Memory access at offset 96 partially overflows this variable > [160, 200) 'save_ip' <== Memory access at offset 96 partially underflows this variable > > The sockaddr_storage pointer is the sockaddr_in6 lhost on the > stack. Copy only the source addr size. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> > --- > slirp/tftp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/slirp/tftp.c b/slirp/tftp.c > index 50e714807d..a9bc4bb1b6 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct sockaddr_storage *srcsas, > > found: > memset(spt, 0, sizeof(*spt)); > - spt->client_addr = *srcsas; > + memcpy(&spt->client_addr, srcsas, sockaddr_size(srcsas)); > spt->fd = -1; > spt->block_size = 512; > spt->client_port = tp->udp.uh_sport; > -- > 2.12.0.191.gc5d8de91d > -- Samuel gawk; talk; nice; date; wine; grep; touch; unzip; strip; \ touch; gasp; finger; gasp; lyx; gasp; latex; mount; fsck; \ more; yes; gasp; umount; make clean; make mrproper; sleep
On 23.03.2017 12:31, Marc-André Lureau wrote: > ASAN detects an "unknown-crash" when running pxe-test: > > /ppc64/pxe/spapr-vlan: ================================================================= > ==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at pc 0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0 > READ of size 128 at 0x7f6dcd298d30 thread T2 > #0 0x55e22218830c in tftp_session_allocate /home/elmarco/src/qq/slirp/tftp.c:73 > #1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289 > #2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446 > #3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82 > #4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67 > > Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame > #0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13 > > This frame has 3 object(s): > [32, 48) '<unknown>' > [96, 124) 'lhost' <== Memory access at offset 96 partially overflows this variable > [160, 200) 'save_ip' <== Memory access at offset 96 partially underflows this variable > > The sockaddr_storage pointer is the sockaddr_in6 lhost on the > stack. Copy only the source addr size. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > slirp/tftp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/slirp/tftp.c b/slirp/tftp.c > index 50e714807d..a9bc4bb1b6 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct sockaddr_storage *srcsas, > > found: > memset(spt, 0, sizeof(*spt)); > - spt->client_addr = *srcsas; > + memcpy(&spt->client_addr, srcsas, sockaddr_size(srcsas)); > spt->fd = -1; > spt->block_size = 512; > spt->client_port = tp->udp.uh_sport; > Makes sense. Reviewed-by: Thomas Huth <thuth@redhat.com>
On 03/23/2017 08:31 AM, Marc-André Lureau wrote: > ASAN detects an "unknown-crash" when running pxe-test: > > /ppc64/pxe/spapr-vlan: ================================================================= > ==7143==ERROR: AddressSanitizer: unknown-crash on address 0x7f6dcd298d30 at pc 0x55e22218830d bp 0x7f6dcd2989e0 sp 0x7f6dcd2989d0 > READ of size 128 at 0x7f6dcd298d30 thread T2 > #0 0x55e22218830c in tftp_session_allocate /home/elmarco/src/qq/slirp/tftp.c:73 > #1 0x55e22218a1f8 in tftp_handle_rrq /home/elmarco/src/qq/slirp/tftp.c:289 > #2 0x55e22218b54c in tftp_input /home/elmarco/src/qq/slirp/tftp.c:446 > #3 0x55e2221833fe in udp6_input /home/elmarco/src/qq/slirp/udp6.c:82 Interesting bug, udp6.c seems to have been started as a copy of udp.c, which uses: struct sockaddr_storage lhost; struct sockaddr_in *lhost4; but in udp6.c this was changed to: struct sockaddr_in6 lhost; tftp_session_allocate() behavior was fine with IPv4 but dual-stack broke it. Your fix uses sockaddr_size() which is the cleaner/safer way to do it but I'm wondering how to avoid such dual-stack sockaddr issues before runtime ASAN spotting. One way can be to use 'union slirp_sockaddr' recently extracted by Dave Gilbert (see http://patchwork.ozlabs.org/patch/746605), it uses more stack but this avoid partial underflows. > #4 0x55e222137b17 in ip6_input /home/elmarco/src/qq/slirp/ip6_input.c:67 > > Address 0x7f6dcd298d30 is located in stack of thread T2 at offset 96 in frame > #0 0x55e222182420 in udp6_input /home/elmarco/src/qq/slirp/udp6.c:13 > > This frame has 3 object(s): > [32, 48) '<unknown>' > [96, 124) 'lhost' <== Memory access at offset 96 partially overflows this variable > [160, 200) 'save_ip' <== Memory access at offset 96 partially underflows this variable > > The sockaddr_storage pointer is the sockaddr_in6 lhost on the > stack. Copy only the source addr size. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > slirp/tftp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/slirp/tftp.c b/slirp/tftp.c > index 50e714807d..a9bc4bb1b6 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -70,7 +70,7 @@ static int tftp_session_allocate(Slirp *slirp, struct sockaddr_storage *srcsas, > > found: > memset(spt, 0, sizeof(*spt)); > - spt->client_addr = *srcsas; > + memcpy(&spt->client_addr, srcsas, sockaddr_size(srcsas)); > spt->fd = -1; > spt->block_size = 512; > spt->client_port = tp->udp.uh_sport; >
© 2016 - 2024 Red Hat, Inc.