#64 netShutdownMulticastIPv4() may attempt to leave group with IP 0x0

[Fredrik Möller]

Seen in ptpd v2.3.0

Problem:
netShutdownMulticastIPv4() does not check whether multicastAddr is 0x0 or not. Hence, setsockopt(IP_DROP_MEMBERSHIP) is called with this invalid address:

static Boolean
netShutdownMulticastIPv4(NetPath * netPath, Integer32 multicastAddr)
{
    struct ip_mreq imr;

    /* Close General Multicast */
    imr.imr_multiaddr.s_addr = multicastAddr;
    imr.imr_interface.s_addr = netPath->interfaceAddr.s_addr;

    setsockopt(netPath->eventSock, IPPROTO_IP, IP_DROP_MEMBERSHIP, 
           &imr, sizeof(struct ip_mreq));
    setsockopt(netPath->generalSock, IPPROTO_IP, IP_DROP_MEMBERSHIP, 
           &imr, sizeof(struct ip_mreq));

    return TRUE;
}

When using the LWIP TCP/IP stack, an assertion is triggered for this condition (unless assertions are disabled).

Proposal:
When entering netShutdownMulticastIPv4(), check if multicastAddr is 0x0. In that case, return immediately.

[Wojciech Owczarek]

Fredrik,

A zero value should not get there in the first place. NetInitMulticast always calls ChooseMcastGroup before netInitMulticastIPv4. After a quick analysis, the only reason I can think of where a zero would get there, is if there was an error while initialising the network.

Under what conditions does this get triggered? We can add the check you suggest, but I'd like to find out why this is happening at all.

I'm quite interested in having this working with LWIP - some FPGA based offload cards use it that are of my interest.

We are planning a major rewrite of the network subsystem in the next version of ptpd anyway, but any bug identified is a help.

Regards
Wojciech

[Wojciech Owczarek]

OK, are you running unicast?

It looks like netShutdownMulticast is called in netShutdown even if we're running unicast - this is one place where this could happen.

Regards
Wojciech

[Fredrik Möller]

No, I'm not running unicast. It's normal IPv4 multicast.

[Jan Breuer]

It is old problem of PTPd design. PTPd calls netShutdown prior to any netInit. Sockets and all netPath variables are uninitialised. It is silently ignored on Linux and FreeBSD but it may cause problems on other platforms: LwIP based, InterrvalZero RTX RTTCP/IP stack, MS Windows, ...

Regards,
Jan

[Fredrik Möller]

Wojciech and Jan,

Thanks for looking at this.

The root cause of the problem is the problem described in [#63], i.e. that netPath.interfaceAddr no longer is the same as the interface IP address. The sequence of events leading to the problem described in the ticket description goes like this:

1. Ethernet link goes up after being down, but with another interface IP address. Hence, netPath.interfaceAddr is now wrong.
2. netRefreshIGMP() is called, which
   • calls netShutdownMulticast(), which sets netpath.multcastAddr and netpath.peerMulticastAddr to zero
   • and then calls netInitMulticast(), which sets netPath.multicastAddr to the correct value but leaves netpath.peerMulticastAddr as zero as netInitMulticast() immediately returns after the failing call to netInitMulticastIPv4(netPath.multicastAddr) (as netPath.interfaceAddr is wrong).
3. Later, netRefreshIGMP() is called again. netpath.peerMulticastAddr remains zero as before, so the call to netShutdownMulticastIPv4(netpath.peerMulticastAddr) fails as described in the ticket description.

Regards,
Fredrik

[Wojciech Owczarek]

Finally fixed this, svn 594