Theo de Raadt <[email protected]> wrote:
> Sebastien Marie <[email protected]> wrote:
>
> > On Sat, Dec 07, 2019 at 11:47:58AM +0100, Sebastien Marie wrote:
> > >
> > > For now, I recompiled slaacd with debug symbols, and will keep it running
> > > like
> > > that in order to catch it the next time. But such switchs aren't very
> > > frequent...
> >
> > A new one, with symbols this time. I still saw it only i386 and aarch64 (not
> > on amd64).
> >
> > $ sysctl kern.version
> > kern.version=OpenBSD 6.6-current (GENERIC.MP) #103: Wed Dec 4 20:10:51 CET
> > 2019
> > [email protected]:/home/openbsd/src/sys/arch/i386/compile/GENERIC.MP
> >
> > $ doas egdb /sbin/slaacd /slaacd.core
> > GNU gdb (GDB) 7.12.1
> > Copyright (C) 2017 Free Software Foundation, Inc.
> > License GPLv3+: GNU GPL version 3 or later
> > <http://gnu.org/licenses/gpl.html>
> > This is free software: you are free to change and redistribute it.
> > There is NO WARRANTY, to the extent permitted by law. Type "show copying"
> > and "show warranty" for details.
> > This GDB was configured as "i386-unknown-openbsd6.5".
> > Type "show configuration" for configuration details.
> > For bug reporting instructions, please see:
> > <http://www.gnu.org/software/gdb/bugs/>.
> > Find the GDB manual and other documentation resources online at:
> > <http://www.gnu.org/software/gdb/documentation/>.
> > For help, type "help".
> > Type "apropos word" to search for commands related to "word"...
> > Reading symbols from /sbin/slaacd...done.
> > [New process 221665]
> >
> > warning: Unexpected size of section `.reg2/221665' in core file.
> > Core was generated by `slaacd'.
> > Program terminated with signal SIGABRT, Aborted.
> >
> > warning: Unexpected size of section `.reg2/221665' in core file.
> > #0 thrkill () at -:3
> > 3 -: No such file or directory.
> > (gdb) bt
> > #0 thrkill () at -:3
> > #1 0x15dcde20 in _libc_raise (s=6) at
> > /home/openbsd/src/lib/libc/gen/raise.c:37
> > #2 0x15dcddab in _libc_abort () at
> > /home/openbsd/src/lib/libc/stdlib/abort.c:51
> > #3 0x15dcdd60 in memcpy (dst0=0xcf7f4c88, src0=0xcf7f4c3c, length=272) at
> > /home/openbsd/src/lib/libc/string/memcpy.c:74
> > #4 0x15d8f1a2 in delete_address (address=<optimized out>) at
> > /home/openbsd/src/sbin/slaacd/slaacd.c:730
> > #5 main_dispatch_engine (fd=<optimized out>, event=2, bula=0x761bd000) at
> > /home/openbsd/src/sbin/slaacd/slaacd.c:534
> > #6 0x15d91058 in event_process_active (base=<optimized out>) at
> > /home/openbsd/src/lib/libevent/event.c:333
> > #7 event_base_loop (base=0x49a33c00, flags=0) at
> > /home/openbsd/src/lib/libevent/event.c:483
> > #8 0x15d90af1 in event_loop (flags=<error reading variable: Cannot access
> > memory at address 0x0>) at /home/openbsd/src/lib/libevent/event.c:409
> > #9 event_dispatch () at /home/openbsd/src/lib/libevent/event.c:347
> > #10 0x15d8e92e in main (argc=<optimized out>, argv=0xcf7f5064) at
> > /home/openbsd/src/sbin/slaacd/slaacd.c:305
> >
> > From frame #3, it means:
> >
> > memcpy(&in6_ridreq.ifr_ifru, &address->addr,
> > sizeof(in6_ridreq.ifr_ifru));
> >
> > &in6_ridreq.ifr_ifru is at 0xcf7f4c88
> > &address->addr is at 0xcf7f4c3c
> > sizeof(in6_ridreq.ifr_ifru) is 272
> >
> > `address' is from the stack in frame #5 (main_dispatch_engine), and
> > `in6_ridreq' is
> > from the stack in frame #4 (delete_address).
> >
> > `address->addr' is a `struct sockaddr_in6' and the sizeof() is 26 bytes.
> > `in6_ridreq.ifr_ifru' is a union with `struct sockaddr_in6' in member, but
> > the
> > total sizeof() is 272 (I assume due to bigger members).
> >
> > so memcpy() will read 272 bytes from the stack, even if the source is only
> > 26
> > bytes long. some bytes will come from outside the variable, specially from
> > the
> > next variable on the stack, and in this case it is from `in6_ridreq' itself,
> > resulting in backwards memcpy.
>
> It should use the specific element in the union, so this might be the
> fix.
>
> Index: slaacd.c
> ===================================================================
> RCS file: /cvs/src/sbin/slaacd/slaacd.c,v
> retrieving revision 1.45
> diff -u -p -u -r1.45 slaacd.c
> --- slaacd.c 23 Nov 2019 08:17:39 -0000 1.45
> +++ slaacd.c 15 Dec 2019 06:31:02 -0000
> @@ -727,8 +727,8 @@ delete_address(struct imsg_configure_add
> return;
> }
>
> - memcpy(&in6_ridreq.ifr_ifru, &address->addr,
> - sizeof(in6_ridreq.ifr_ifru));
> + memcpy(&in6_ridreq.ifr_ifru.ifru_addr, &address->addr,
> + sizeof(in6_ridreq.ifr_ifru.ifru_addr));
>
> log_debug("%s: %s", __func__, if_name);
Or even better, via the typical macro expansion used for this type of
union, it should use ifr_addr
Index: slaacd.c
===================================================================
RCS file: /cvs/src/sbin/slaacd/slaacd.c,v
retrieving revision 1.45
diff -u -p -u -r1.45 slaacd.c
--- slaacd.c 23 Nov 2019 08:17:39 -0000 1.45
+++ slaacd.c 15 Dec 2019 07:00:47 -0000
@@ -727,8 +727,8 @@ delete_address(struct imsg_configure_add
return;
}
- memcpy(&in6_ridreq.ifr_ifru, &address->addr,
- sizeof(in6_ridreq.ifr_ifru));
+ memcpy(&in6_ridreq.ifr_addr, &address->addr,
+ sizeof(in6_ridreq.ifr_addr));
log_debug("%s: %s", __func__, if_name);
