Re: Regression on rtadvd (was: Re: CVS commit: src/usr.sbin/rtadvd)

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Mon, Jun 15, 2015 at 5:58 AM, Timo Buhrmester <fstd.lkml%gmail.com@localhost> wrote:
>> Module Name:  src
>> Committed By: roy
>> Date:         Fri Jun  5 14:15:41 UTC 2015
>>
>> Modified Files:
>>       src/usr.sbin/rtadvd: rtadvd.c
>>
>> Log Message:
>> Set the hoplimit of 255 as specified in RFC 4861 section 4.2
>> using the IPV6_MULTICAST_HOPS socket option rather than using CMSG
>> when constructing each message.
> This commit broke rtadvd for me, on i386, which now gives "Invalid argument" on the call to sendmsg() that is supposed to generate RAs.
>
> Here's two gdb transcripts:
>
> This is what happens AFTER the offending commit:
>
> | # gdb -q rtadvd
> | (gdb) break rtadvd.c:1699
> | (gdb) run -df vr1
> | [...]
> | Breakpoint 1, ra_output (rai=0xbb91c0e0) at /usr/src.head/usr.sbin/rtadvd/rtadvd.c:1699
> | 1699          i = sendmsg(sock, &sndmhdr, 0);
> | (gdb) bt
> | #0  ra_output (rai=0xbb91c0e0) at /usr/src.head/usr.sbin/rtadvd/rtadvd.c:1699
> | #1  0x0804cdbf in ra_timeout (data=0xbb91c0e0) at /usr/src.head/usr.sbin/rtadvd/rtadvd.c:1779
> | #2  0x08052610 in rtadvd_check_timer () at /usr/src.head/usr.sbin/rtadvd/timer.c:130
> | #3  0x080499c0 in main (argc=-1, argv=0xbfbfec64) at /usr/src.head/usr.sbin/rtadvd/rtadvd.c:315
> | (gdb) x/28xb &sndmhdr
> | 0x8058b2c <sndmhdr>:    0x40  0x7e    0x05    0x08    0x1c    0x00    0x00    0x00
> | 0x8058b34 <sndmhdr+8>:  0xc4  0x8a    0x05    0x08    0x01    0x00    0x00    0x00
> | 0x8058b3c <sndmhdr+16>: 0xd0  0xa0    0x90    0xbb    0x30    0x00    0x00    0x00
> | 0x8058b44 <sndmhdr+24>: 0x00  0x00    0x00    0x00
> | (gdb) n
> | 1701          if (i < 0 || (size_t)i != rai->ra_datalen)  {
> | (gdb) print i
> | $1 = -1
> | (gdb) n
> | [...]
> | rtadvd[3794]: <ra_output> sendmsg on vr1: Invalid argument
>
>
>
> The following is what it looked like BEFORE the offending commit:
>
> | # gdb -q rtadvd
> | (gdb) break rtadvd.c:1702
> | (gdb) run -df vr1
> | [...]
> | Breakpoint 1, ra_output (rai=0xbb91c0e0) at /usr/src.head/usr.sbin/rtadvd/rtadvd.c:1702
> | 1702          i = sendmsg(sock, &sndmhdr, 0);
> | (gdb) bt
> | #0  ra_output (rai=0xbb91c0e0) at /usr/src.head/usr.sbin/rtadvd/rtadvd.c:1702
> | #1  0x0804cdcf in ra_timeout (data=0xbb91c0e0) at /usr/src.head/usr.sbin/rtadvd/rtadvd.c:1782
> | #2  0x08052620 in rtadvd_check_timer () at /usr/src.head/usr.sbin/rtadvd/timer.c:130
> | #3  0x080499c0 in main (argc=-1, argv=0xbfbfec64) at /usr/src.head/usr.sbin/rtadvd/rtadvd.c:315
> | (gdb) x/28xb &sndmhdr
> | 0x8058b0c <sndmhdr>:    0x20  0x7e    0x05    0x08    0x1c    0x00    0x00    0x00
> | 0x8058b14 <sndmhdr+8>:  0xa4  0x8a    0x05    0x08    0x01    0x00    0x00    0x00
> | 0x8058b1c <sndmhdr+16>: 0xd0  0xa0    0x90    0xbb    0x30    0x00    0x00    0x00
> | 0x8058b24 <sndmhdr+24>: 0x00  0x00    0x00    0x00
> | (gdb) n
> | 1704          if (i < 0 || (size_t)i != rai->ra_datalen)  {
> | (gdb) print i
> | $2 = 56
>
>
>
> The difference in the representations of `sndmhdr` are in the 1st and 9th byte, which on i386 correspond to the 1st and 3rd members of struct sndhdr:
> |        void            *msg_name;      /* optional address */
> | and    struct iovec    *msg_iov;       /* scatter/gather array */
>
>
> I've manually tracked the failing code path down through the sendmsg system call:
> sys/kern/uipc_syscalls.c:620 do_sys_sendmsg_so()  ``error = (*so->so_send)(so, sa, &auio, NULL, control, flags, l);''
> sys/kern/uipc_socket.c:1602 sosend()              ``error = (*so->so_proto->pr_usrreqs->pr_send)(so, top, addr, control, l);''
> sys/netinet6/raw_ip6.c:891 rip6_send()            ``error = rip6_output(m, so, dst, control);''
> sys/netinet6/raw_ip6.c:391 rip6_output()          ``if ((error = ip6_setpktopts(control, &opt, [...]''
> sys/netinet6/ip6_output.c:2705 ip6_setpktopts()   ``if (cm->cmsg_len == 0 || cm->cmsg_len > control->m_len)''
>
> The last function is where the EINVAL is produced, due to cm->cmsg_len being zero (in the 2nd iteration of the enclosing loop)
> Unfortunately, I couldn't figure out what's supposed to happen there.
>
>
> Any ideas?

It seems that the kernel attempts to read non-existent cmsg
which was removed by the commit. Can you try the below patch?
It reduces cmsg buffer length as hoplimit cmsg is removed.

  ozaki-r

diff --git a/usr.sbin/rtadvd/rtadvd.c b/usr.sbin/rtadvd/rtadvd.c
index 66885c4..f2016eb 100644
--- a/usr.sbin/rtadvd/rtadvd.c
+++ b/usr.sbin/rtadvd/rtadvd.c
@@ -1503,8 +1503,7 @@ sock_open(void)
                exit(1);
        }

-       sndcmsgbuflen = CMSG_SPACE(sizeof(struct in6_pktinfo)) +
-                               CMSG_SPACE(sizeof(int));
+       sndcmsgbuflen = CMSG_SPACE(sizeof(struct in6_pktinfo));
        sndcmsgbuf = malloc(sndcmsgbuflen);
        if (sndcmsgbuf == NULL) {
                syslog(LOG_ERR, "<%s> malloc: %m", __func__);