UDP checksum calculation not working with newer version of gcc-CodePudding

The code included below is a stripped-down implementation of a function that generates a UDP packet for a given fixed-size payload and sends it.

After switching to a newer version of gcc this code suddenly shows an error: the UDP checksum is not calculated correctly, and this can be traced down to the line

pseudoHeader->protocol = IPPROTO_UDP;

for which the compiler seemingly does not generate an instruction if at least -O2 optimization is used.

The following workarounds resolve the issue (each suggestion works independently, i.e. you do not to have to apply all of them at once!):

move the mentioned line before the two calls to inet_pton
remove the call to memset(ipHeader, 0, sizeof(struct ip)) after the checksum calculation
make ip_checksum() an external function outside of this translation unit

The fact that the code makes heavy use of casting together with the error only appearing for -O2 or higher and the nature of the workarounds virtually calls for this to be an aliasing error in the code. Is there an actual error and if so, how can it be fixed?

#include <string.h>
#include <arpa/inet.h>
#include <netinet/ip.h>
#include <netinet/udp.h>
#include <netpacket/packet.h>

#define UDP_PORT 2345
#define REPLY_PAYLOAD_SIZE 360

typedef struct UDPPseudoHeader
{
    unsigned long int source_ip;
    unsigned long int dest_ip;
    unsigned char reserved;
    unsigned char protocol;
    unsigned short int udp_length;
} UDPPseudoHeader;

void sendPacket(unsigned char* packet, int len);

static unsigned short ip_checksum(unsigned short *ptr, int len)
{
    int sum = 0;
    unsigned short answer = 0;
    unsigned short *w = ptr;
    int nleft = len;

    while(nleft > 1) {
        sum  = *w  ;
        nleft -= 2;
    }

    sum = (sum >> 16)   (sum & 0xFFFF);
    sum  = (sum >> 16);
    answer = ~sum;
    return(answer);
}

void sendBroadcastPacket(uint16_t destPort, char* packet) {
    unsigned char buffer[REPLY_PAYLOAD_SIZE   sizeof(struct ip)   sizeof(struct udphdr)];
    int bufferLen = REPLY_PAYLOAD_SIZE   sizeof(struct ip)   sizeof(struct udphdr);

    /* initialize header pointers */
    struct udphdr* udpHeader = (struct udphdr*)(buffer   sizeof(struct ip));
    UDPPseudoHeader* pseudoHeader = (UDPPseudoHeader*)(buffer   sizeof(struct ip) - sizeof(UDPPseudoHeader));
    struct ip* ipHeader = (struct ip*)(buffer);

    memset(buffer, 0, bufferLen);

    /* copy user data */
    memcpy(buffer   sizeof(struct ip)   sizeof(struct udphdr), packet, REPLY_PAYLOAD_SIZE);

    /* fill in UDP header */
    udpHeader->source = htons(UDP_PORT);
    udpHeader->dest = htons(destPort);
    udpHeader->len = htons(sizeof(struct udphdr)   REPLY_PAYLOAD_SIZE);
    udpHeader->check = 0;

    /* create UDP pseudo header for checksum calculation */
    inet_pton(AF_INET, "0.0.0.0", &pseudoHeader->source_ip);
    inet_pton(AF_INET, "255.255.255.255", &pseudoHeader->dest_ip);
    pseudoHeader->reserved = 0;
    pseudoHeader->protocol = IPPROTO_UDP;
    pseudoHeader->udp_length = htons(sizeof(struct udphdr)   REPLY_PAYLOAD_SIZE);

    /* calculate UDP checksum */
    udpHeader->check = ip_checksum((unsigned short*) pseudoHeader, bufferLen - sizeof(struct ip)   sizeof(UDPPseudoHeader));

    /* fill in IP header */
    memset(ipHeader, 0, sizeof(struct ip));
    ipHeader->ip_v = 4;
    ipHeader->ip_hl = 5;
    ipHeader->ip_tos = IPTOS_LOWDELAY;
    ipHeader->ip_len = htons(bufferLen);
    ipHeader->ip_off = htons(IP_DF);
    ipHeader->ip_id = 0;
    ipHeader->ip_ttl = 16;
    ipHeader->ip_p = IPPROTO_UDP;
    inet_pton(AF_INET, "0.0.0.0", &ipHeader->ip_src);
    inet_pton(AF_INET, "255.255.255.255", &ipHeader->ip_dst);
    ipHeader->ip_sum = 0;

    /* calculate IP checksum */
    ipHeader->ip_sum = ip_checksum((unsigned short*) ipHeader, ipHeader->ip_hl * 4);

    sendPacket(buffer, bufferLen);
}

CodePudding user response：

The code indeed violates the strict aliasing rule. The compiler assumes that the call to ip_checksum() does not depend on the assignments to the struct members reserved and protocol because these modify chars and ip_checksum() is calculated over an array of unsigned shorts. Therefore the assignments are completely optimized away since the following call to memset() overwrites the memory anyway.

A possible solution is to declare the pseudo header as

typedef union {
    struct {
        unsigned long int source_ip;
        unsigned long int dest_ip;
        unsigned char reserved;
        unsigned char protocol;
        unsigned short int udp_length;
    } hdr;
    unsigned short as_short[6];
} UDPPseudoHeader;

and replace the generation of the pseudo header and the checksum calculation by

/* create UDP pseudo header for checksum calculation */
inet_pton(AF_INET, "0.0.0.0", &pseudoHeader->hdr.source_ip);
inet_pton(AF_INET, "255.255.255.255", &pseudoHeader->hdr.dest_ip);
pseudoHeader->hdr.reserved = 0;
pseudoHeader->hdr.protocol = IPPROTO_UDP;
pseudoHeader->hdr.udp_length = htons(sizeof(struct udphdr)   REPLY_PAYLOAD_SIZE);

/* calculate UDP checksum */
udpHeader->check = ip_checksum(pseudoHeader->as_short, bufferLen - sizeof(struct ip)   sizeof(UDPPseudoHeader));

CodePudding user response：

Another issue:

Alignment

unsigned char buffer[REPLY_PAYLOAD_SIZE   sizeof(struct ip)   sizeof(struct udphdr)];
...
struct ip* ipHeader = (struct ip*)(buffer);

buffer is not certainly aligned for a struct ip.