[PATCH] netfilter ulog byte alignment problem

[PATCH] netfilter ulog byte alignment problem

[Fabian Hugelshofer]

I am using libipulog on an XScale IXP422 processor (big endian ARM). I
call ipulog_get_packet to decode the multipart message and then access
the fields in the packet (ulog_packet_msg_t). Accessing fields 'mac' and
'payload' does not give me the right data. The data is actually there,
but an access to the arrays ends up at a wrong location. Manually adding
an offset leads to the correct position (e.g. pkt->payload[-66] instead
of pkt->payload[0] for the first byte of the IP packet).

I tracked the issue down to a byte alignment problem in
ulog_packet_msg_t (include/linux/netfilter_ipv4/ipt_ULOG.h). Setting
attributes 'packed' or 'aligned' or using 'size_t' instead of 'unsigned
char' for field 'mac_len' solved the problem. It seems necessary to help
the compiler doing things right.

This patch requires applications to be recompiled against the new header
file. Let me know what you think.

linux 2.6.25.4
libipulog r6382
armeb-linux-gcc 4.2.4
CFLAGS -pipe -march=armv5te -mtune=xscale -funit-at-a-time
-fhonour-copts -msoft-float

Signed-off-by: Fabian Hugelshofer <hugelshofer2006@gmx.ch>

--- linux-2.6.25.4/include/linux/netfilter_ipv4/ipt_ULOG.h.orig	2008-06-02 15:39:22.000000000 +0100
+++ linux-2.6.25.4/include/linux/netfilter_ipv4/ipt_ULOG.h	2008-06-02 15:36:34.000000000 +0100
@@ -44,6 +44,6 @@
 	unsigned char mac_len;
 	unsigned char mac[ULOG_MAC_LEN];
 	unsigned char payload[0];
-} ulog_packet_msg_t;
+} ulog_packet_msg_t __attribute__ ((packed));
 
 #endif /*_IPT_ULOG_H*/


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter ulog byte alignment problem

[Patrick McHardy]

Fabian Hugelshofer wrote:
> I am using libipulog on an XScale IXP422 processor (big endian ARM). I
> call ipulog_get_packet to decode the multipart message and then access
> the fields in the packet (ulog_packet_msg_t). Accessing fields 'mac' and
> 'payload' does not give me the right data. The data is actually there,
> but an access to the arrays ends up at a wrong location. Manually adding
> an offset leads to the correct position (e.g. pkt->payload[-66] instead
> of pkt->payload[0] for the first byte of the IP packet).
> 
> I tracked the issue down to a byte alignment problem in
> ulog_packet_msg_t (include/linux/netfilter_ipv4/ipt_ULOG.h). Setting
> attributes 'packed' or 'aligned' or using 'size_t' instead of 'unsigned
> char' for field 'mac_len' solved the problem. It seems necessary to help
> the compiler doing things right.
> 
> This patch requires applications to be recompiled against the new header
> file. Let me know what you think.

I'm afraid it might break compatibility on other architectures.
Are your userspace and kernel compiled using the same options?
Why would they lay out the structure differently?

Re: [PATCH] netfilter ulog byte alignment problem

[Fabian Hugelshofer]

Patrick McHardy wrote:
> Fabian Hugelshofer wrote:
>> I tracked the issue down to a byte alignment problem in
>> ulog_packet_msg_t (include/linux/netfilter_ipv4/ipt_ULOG.h). Setting
>> attributes 'packed' or 'aligned' or using 'size_t' instead of 'unsigned
>> char' for field 'mac_len' solved the problem. It seems necessary to help
>> the compiler doing things right.
[...]
> I'm afraid it might break compatibility on other architectures.
> Are your userspace and kernel compiled using the same options?
> Why would they lay out the structure differently?

I think you are right. The compiler lays it out consistantly while
respecting proper alignment. I removed my patch again and it worked
event without. I think the reason was header files which were out of 
sync (kernel vs. userspace). I had to modify ipt_ULOG.(c|h) for a driver 
problem workaround. I'm quire sure that I reverted the changes during 
the testing, but maybe I missed that.

I would write again, if the problem reappears later.