[PATCH 2/3] libnetlink-log byte alignment

[PATCH 2/3] libnetlink-log byte alignment

[Fabian Hugelshofer]

Aligns buffers to maximum alignment of architecture to make the cast of
char pointers to struct pointers more portable. Packet decoding is still
broken on particular platforms.

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


diff -ruN libnfnetlink-0.0.38.orig/src/libnfnetlink.c libnfnetlink-0.0.38/src/libnfnetlink.c
--- libnfnetlink-0.0.38.orig/src/libnfnetlink.c	2008-06-02 18:54:18.000000000 +0100
+++ libnfnetlink-0.0.38/src/libnfnetlink.c	2008-06-02 18:57:32.000000000 +0100
@@ -528,7 +528,7 @@
 			       void *), void *jarg)
 {
 	struct sockaddr_nl nladdr;
-	char buf[NFNL_BUFFSIZE];
+	char buf[NFNL_BUFFSIZE] __attribute__ ((aligned));
 	struct iovec iov;
 	int remain;
 	struct nlmsghdr *h;
@@ -637,7 +637,7 @@
 	      int (*junk)(struct sockaddr_nl *, struct nlmsghdr *n, void *),
 	      void *jarg)
 {
-	char buf[NFNL_BUFFSIZE];
+	char buf[NFNL_BUFFSIZE] __attribute__ ((aligned));
 	struct sockaddr_nl nladdr;
 	struct nlmsghdr *h;
 	unsigned int seq;
@@ -1474,7 +1474,8 @@
 	assert(h);
 
 	while (1) {
-		unsigned char buf[h->rcv_buffer_size];
+		unsigned char buf[h->rcv_buffer_size]
+			__attribute__ ((aligned));
 
 		ret = nfnl_recv(h, buf, sizeof(buf));
 		if (ret == -1) {

Re: [PATCH 2/3] libnetlink-log byte alignment

[Fabian Hugelshofer]

Use union of char buffer and message header to ensure proper byte
alignment.

This patch increases portability, but alignment is still not respected
in nflog_get_*() which still leaves libnetfilter_log unusable on
particular platforms.

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

diff --git a/src/libnetfilter_log.c b/src/libnetfilter_log.c
index a3bd435..2019b0c 100644
--- a/src/libnetfilter_log.c
+++ b/src/libnetfilter_log.c
@@ -106,18 +106,20 @@ static int
 __build_send_cfg_msg(struct nflog_handle *h, u_int8_t command,
 		     u_int16_t queuenum, u_int8_t pf)
 {
-	char buf[NFNL_HEADER_LEN
-		+NFA_LENGTH(sizeof(struct nfulnl_msg_config_cmd))];
+	union {
+		char buf[NFNL_HEADER_LEN
+			+NFA_LENGTH(sizeof(struct nfulnl_msg_config_cmd))];
+		struct nlmsghdr nmh;
+	} u;
 	struct nfulnl_msg_config_cmd cmd;
-	struct nlmsghdr *nmh = (struct nlmsghdr *) buf;
 
-	nfnl_fill_hdr(h->nfnlssh, nmh, 0, pf, queuenum,
+	nfnl_fill_hdr(h->nfnlssh, &u.nmh, 0, pf, queuenum,
 		      NFULNL_MSG_CONFIG, NLM_F_REQUEST|NLM_F_ACK);
 
 	cmd.command = command;
-	nfnl_addattr_l(nmh, sizeof(buf), NFULA_CFG_CMD, &cmd, sizeof(cmd));
+	nfnl_addattr_l(&u.nmh, sizeof(u), NFULA_CFG_CMD, &cmd, sizeof(cmd));
 
-	return nfnl_talk(h->nfnlh, nmh, 0, 0, NULL, NULL, NULL);
+	return nfnl_talk(h->nfnlh, &u.nmh, 0, 0, NULL, NULL, NULL);
 }
 
 static int __nflog_rcv_pkt(struct nlmsghdr *nlh, struct nfattr *nfa[],
@@ -291,60 +293,68 @@ int nflog_unbind_group(struct nflog_g_handle *gh)
 int nflog_set_mode(struct nflog_g_handle *gh,
 		   u_int8_t mode, u_int32_t range)
 {
-	char buf[NFNL_HEADER_LEN
-		+NFA_LENGTH(sizeof(struct nfulnl_msg_config_mode))];
+	union {
+		char buf[NFNL_HEADER_LEN
+			+NFA_LENGTH(sizeof(struct nfulnl_msg_config_mode))];
+		struct nlmsghdr nmh;
+	} u;
 	struct nfulnl_msg_config_mode params;
-	struct nlmsghdr *nmh = (struct nlmsghdr *) buf;
 
-	nfnl_fill_hdr(gh->h->nfnlssh, nmh, 0, AF_UNSPEC, gh->id,
+	nfnl_fill_hdr(gh->h->nfnlssh, &u.nmh, 0, AF_UNSPEC, gh->id,
 		      NFULNL_MSG_CONFIG, NLM_F_REQUEST|NLM_F_ACK);
 
 	params.copy_range = htonl(range);	/* copy_range is short */
 	params.copy_mode = mode;
-	nfnl_addattr_l(nmh, sizeof(buf), NFULA_CFG_MODE, &params,
+	nfnl_addattr_l(&u.nmh, sizeof(u), NFULA_CFG_MODE, &params,
 		       sizeof(params));
 
-	return nfnl_talk(gh->h->nfnlh, nmh, 0, 0, NULL, NULL, NULL);
+	return nfnl_talk(gh->h->nfnlh, &u.nmh, 0, 0, NULL, NULL, NULL);
 }
 
 int nflog_set_timeout(struct nflog_g_handle *gh, u_int32_t timeout)
 {
-	char buf[NFNL_HEADER_LEN+NFA_LENGTH(sizeof(u_int32_t))];
-	struct nlmsghdr *nmh = (struct nlmsghdr *) buf;
+	union {
+		char buf[NFNL_HEADER_LEN+NFA_LENGTH(sizeof(u_int32_t))];
+		struct nlmsghdr nmh;
+	} u;
 
-	nfnl_fill_hdr(gh->h->nfnlssh, nmh, 0, AF_UNSPEC, gh->id,
+	nfnl_fill_hdr(gh->h->nfnlssh, &u.nmh, 0, AF_UNSPEC, gh->id,
 		      NFULNL_MSG_CONFIG, NLM_F_REQUEST|NLM_F_ACK);
 
-	nfnl_addattr32(nmh, sizeof(buf), NFULA_CFG_TIMEOUT, htonl(timeout));
+	nfnl_addattr32(&u.nmh, sizeof(u), NFULA_CFG_TIMEOUT, htonl(timeout));
 
-	return nfnl_talk(gh->h->nfnlh, nmh, 0, 0, NULL, NULL, NULL);
+	return nfnl_talk(gh->h->nfnlh, &u.nmh, 0, 0, NULL, NULL, NULL);
 }
 
 int nflog_set_qthresh(struct nflog_g_handle *gh, u_int32_t qthresh)
 {
-	char buf[NFNL_HEADER_LEN+NFA_LENGTH(sizeof(u_int32_t))];
-	struct nlmsghdr *nmh = (struct nlmsghdr *) buf;
+	union {
+		char buf[NFNL_HEADER_LEN+NFA_LENGTH(sizeof(u_int32_t))];
+		struct nlmsghdr nmh;
+	} u;
 
-	nfnl_fill_hdr(gh->h->nfnlssh, nmh, 0, AF_UNSPEC, gh->id,
+	nfnl_fill_hdr(gh->h->nfnlssh, &u.nmh, 0, AF_UNSPEC, gh->id,
 		      NFULNL_MSG_CONFIG, NLM_F_REQUEST|NLM_F_ACK);
 
-	nfnl_addattr32(nmh, sizeof(buf), NFULA_CFG_QTHRESH, htonl(qthresh));
+	nfnl_addattr32(&u.nmh, sizeof(u), NFULA_CFG_QTHRESH, htonl(qthresh));
 
-	return nfnl_talk(gh->h->nfnlh, nmh, 0, 0, NULL, NULL, NULL);
+	return nfnl_talk(gh->h->nfnlh, &u.nmh, 0, 0, NULL, NULL, NULL);
 }
 
 int nflog_set_nlbufsiz(struct nflog_g_handle *gh, u_int32_t nlbufsiz)
 {
-	char buf[NFNL_HEADER_LEN+NFA_LENGTH(sizeof(u_int32_t))];
-	struct nlmsghdr *nmh = (struct nlmsghdr *) buf;
+	union {
+		char buf[NFNL_HEADER_LEN+NFA_LENGTH(sizeof(u_int32_t))];
+		struct nlmsghdr nmh;
+	} u;
 	int status;
 
-	nfnl_fill_hdr(gh->h->nfnlssh, nmh, 0, AF_UNSPEC, gh->id,
+	nfnl_fill_hdr(gh->h->nfnlssh, &u.nmh, 0, AF_UNSPEC, gh->id,
 		      NFULNL_MSG_CONFIG, NLM_F_REQUEST|NLM_F_ACK);
 
-	nfnl_addattr32(nmh, sizeof(buf), NFULA_CFG_NLBUFSIZ, htonl(nlbufsiz));
+	nfnl_addattr32(&u.nmh, sizeof(u), NFULA_CFG_NLBUFSIZ, htonl(nlbufsiz));
 
-	status = nfnl_talk(gh->h->nfnlh, nmh, 0, 0, NULL, NULL, NULL);
+	status = nfnl_talk(gh->h->nfnlh, &u.nmh, 0, 0, NULL, NULL, NULL);
 
 	/* we try to have space for at least 10 messages in the socket buffer */
 	if (status >= 0)
@@ -355,15 +365,17 @@ int nflog_set_nlbufsiz(struct nflog_g_handle *gh, u_int32_t nlbufsiz)
 
 int nflog_set_flags(struct nflog_g_handle *gh, u_int16_t flags)
 {
-	char buf[NFNL_HEADER_LEN+NFA_LENGTH(sizeof(u_int16_t))];
-	struct nlmsghdr *nmh = (struct nlmsghdr *) buf;
+	union {
+		char buf[NFNL_HEADER_LEN+NFA_LENGTH(sizeof(u_int16_t))];
+		struct nlmsghdr nmh;
+	} u;
 
-	nfnl_fill_hdr(gh->h->nfnlssh, nmh, 0, AF_UNSPEC, gh->id,
+	nfnl_fill_hdr(gh->h->nfnlssh, &u.nmh, 0, AF_UNSPEC, gh->id,
 		      NFULNL_MSG_CONFIG, NLM_F_REQUEST|NLM_F_ACK);
 
-	nfnl_addattr16(nmh, sizeof(buf), NFULA_CFG_FLAGS, htons(flags));
+	nfnl_addattr16(&u.nmh, sizeof(u), NFULA_CFG_FLAGS, htons(flags));
 
-	return nfnl_talk(gh->h->nfnlh, nmh, 0, 0, NULL, NULL, NULL);
+	return nfnl_talk(gh->h->nfnlh, &u.nmh, 0, 0, NULL, NULL, NULL);
 }
 
 

Re: [PATCH 2/3] libnetlink-log byte alignment

[Pablo Neira Ayuso]

Fabian Hugelshofer wrote:
> Use union of char buffer and message header to ensure proper byte
> alignment.

Applied. Thanks.

> This patch increases portability, but alignment is still not respected
> in nflog_get_*() which still leaves libnetfilter_log unusable on
> particular platforms.

Sorry, I lost track of this alignment issues, what's problem?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [PATCH 2/3] libnetlink-log byte alignment

[Fabian Hugelshofer]

Pablo Neira Ayuso wrote:
> Fabian Hugelshofer wrote:
>> This patch increases portability, but alignment is still not respected
>> in nflog_get_*() which still leaves libnetfilter_log unusable on
>> particular platforms.
> 
> Sorry, I lost track of this alignment issues, what's problem?

I still have problems running the nflog test utility nfunl_test. I get a 
segmentation fault when nflog_get_payload() is called as soon as the 
first packet is received. The other nflog_get* functions which are 
called before work fine (even though I did not check what they return). 
I aligned the receive buffer and actually don't see the reason for the 
segfault.

When I use libnetfilter_log in my own application it works fine as far 
as I can tell. Byte alignment problems are quite non-deterministic and 
the problem with nfunl_test indicates, that there is still a problem around.