avahi-core socket.c's input cmsghdr msg->msg_control buffers are misaligned on ia64 linux

[Jason Vas Dias <jvdias@…>]

On ia64 linux platforms, the kernel detects and complains about mis-aligned memory accesses, which avahi-daemon upto and including 0.6.6 suffers from:

$ avahi-daemon ... ... avahi-daemon(28929): unaligned access to 0x60000fffffeef2c4, ip=0x20000000000b20c1 avahi-daemon(28929): unaligned access to 0x60000fffffeef2e4, ip=0x20000000000b1ec0 avahi-daemon(28929): unaligned access to 0x60000fffffeef2e4, ip=0x20000000000b2061 avahi-daemon(28929): unaligned access to 0x60000fffffeef2c4, ip=0x20000000000b20c1

Running with: $ prctl --unaligned=signal gdb avahi-daemon

shows the fault occurs in avahi-core's socket.c's avahi_recv_dns_packet_ipv4(...), upon the first invocation of CMSG_NEXTHDR, @line 657 .

It turns out that the 'aux' buffer on the stack, declared as :

uint8_t aux[1024];

was not on an 8-byte boundary, so the first cmsg address (0x60000fffffeef2c4) is not properly aligned for access to a cmsghdr structure.

The following patch fixes the issue wherever it might occur in socket.c, and prevents the messages being generated with Red Hat Fedora Core 5's avahi-0.6.6-1 package . This problem was first reported as Red Hat Bugzilla #179448:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=179448

Please consider fixing this problem in the next release - thanks!

Jason Vas Dias<jvdias@…>, avahi package maintainer Red Hat, Inc.

_BEGIN PATCH_ --- avahi-0.6.5/avahi-core/socket.c.bz179448 2005-11-09 13:45:51.000000000 -0500 +++ avahi-0.6.5/avahi-core/socket.c 2006-02-01 17:54:22.000000000 -0500 @@ -456,10 +456,10 @@

struct iovec io;

#ifdef IP_PKTINFO

struct cmsghdr *cmsg;

- uint8_t cmsg_data[CMSG_SPACE(sizeof(struct in_pktinfo))]; + struct cmsghdr cmsg_data[( CMSG_SPACE(sizeof(struct in_addr)) / sizeof(struct cmsghdr)) + 1];

#elif defined(IP_SENDSRCADDR)

struct cmsghdr *cmsg;

- uint8_t cmsg_data[CMSG_SPACE(sizeof(struct in_addr))]; + struct cmsghdr cmsg_data[( CMSG_SPACE(sizeof(struct in_addr)) / sizeof(struct cmsghdr)) + 1];

#endif

assert(fd >= 0);

@@ -542,7 +542,7 @@

struct msghdr msg; struct iovec io; struct cmsghdr *cmsg;

- uint8_t cmsg_data[CMSG_SPACE(sizeof(struct in6_pktinfo))]; + struct cmsghdr cmsg_data[(CMSG_SPACE(sizeof(struct in6_pktinfo))/sizeof(struct msghdr)) + 1];

assert(fd >= 0); assert(p);

@@ -596,7 +596,7 @@

AvahiDnsPacket? *p= NULL; struct msghdr msg; struct iovec io;

- uint8_t aux[1024]; + struct cmsghdr aux[1024 / sizeof(struct cmsghdr)]; /* for alignment on ia64 ! */

ssize_t l; struct cmsghdr *cmsg; int found_addr = 0;

@@ -726,7 +726,7 @@

AvahiDnsPacket? *p = NULL; struct msghdr msg; struct iovec io;

- uint8_t aux[64]; + struct cmsghdr aux[1024 / sizeof(struct cmsghdr)];

ssize_t l; int ms; struct cmsghdr *cmsg;

_END PATCH_

[Jason Vas Dias <jvdias@…>]

Patch against 0.6.5/6 socket.c to fix issue

[lennart]

Mmm, this patch contains two errors and introduces a new compiler warning I don't understand.

The two errors: once you mistyped "cmsghdr" as "msghdr". And once you wrote "in_addr" instead of "in_pktinfo".

When I apply the patch i Get the followign compiler warnings:

socket.c: In function 'avahi_send_dns_packet_ipv4': socket.c:459: warning: invalid use of structure with flexible array member socket.c: In function 'avahi_send_dns_packet_ipv6': socket.c:545: warning: invalid use of structure with flexible array member socket.c: In function 'avahi_recv_dns_packet_ipv4': socket.c:599: warning: invalid use of structure with flexible array member socket.c: In function 'avahi_recv_dns_packet_ipv6': socket.c:729: warning: invalid use of structure with flexible array member

I wonder what they mean, do you have any idea and could supply me with a patch that doesn't produce these warnnings?

Thank you very much,

Lennart

[Jason Vas Dias <jvdias@…>]

updated patch to fix issue

[Jason Vas Dias<jvdias@…>]

OK, I fixed the typos with the updated patch just uploaded.

The compiler warnings are generated ONLY with the '-pedantic' gcc argument, and occur because the cmsghdr structures are declared:

struct cmsghdr

{

size_t cmsg_len;

int cmsg_level; int cmsg_type;

extension unsigned char cmsg_data [];

};

ie. they end with a "flexible array" of zero size, so in -pedantic mode, the compiler will complain about a vector of such structures. You can avoid this declaration using the -ansi gcc flag.

Possibly using 'size_t' instead of 'struct cmsghdr' for the type of the array would work just as well - the important thing is that the compiler must allocate an address on an 8-byte boundary for the buffers - ie. you could do:

size_t aux[1024 / sizeof(size_t)]; /* for alignment on ia64 ! */

and that would ensure that the array gets an 8-byte aligned address, since size_t is a U64 on ia64 .

[Jason Vas Dias<jvdias@…>]

Updated patch using size_t instead of 'struct cmsghdr' for buffers

[Jason Vas Dias<jvdias@…>]

OK, I just uploaded the latest version of the patch, which declares the buffers as 'size_t' vectors instead of 'struct cmsghdr' vectors, and tested the new patched avahi-daemon on ia64 - no unaligned access faults occurred, and the 'invalid use of structure with flexible array member' warnings have disappeared.

When you compile socket.c on ia64, you still get these warnings with -pedantic:

socket.c: In function 'avahi_send_dns_packet_ipv4': socket.c:498: warning: cast increases required alignment of target type socket.c: In function 'avahi_send_dns_packet_ipv6': socket.c:577: warning: cast increases required alignment of target type socket.c: In function 'avahi_recv_dns_packet_ipv4': socket.c:667: warning: cast increases required alignment of target type socket.c:673: warning: cast increases required alignment of target type socket.c: In function 'avahi_recv_dns_packet_ipv6': socket.c:786: warning: cast increases required alignment of target type socket.c:793: warning: cast increases required alignment of target type

These are for all expressions such as on line 498:

pkti = (struct in_pktinfo*) CMSG_DATA(cmsg);

But now the cmsghdr buffers are aligned correctly on 8 byte boundaries, these warnings do not matter.

[lennart]

I commited this in revision 1130.

Thank you for your contribution!