b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] Possible page faults at route.c
@ 2009-05-20 11:33 Sven Eckelmann
  2009-05-20 11:33 ` [B.A.T.M.A.N.] [PATCH 1/2] [batman] Reserve enough space for aligned netlink requests Sven Eckelmann
  2009-05-20 11:33 ` [B.A.T.M.A.N.] [PATCH 2/2] [batman] Don't add size for netlink header twice in netlink request Sven Eckelmann
  0 siblings, 2 replies; 5+ messages in thread
From: Sven Eckelmann @ 2009-05-20 11:33 UTC (permalink / raw)
  To: b.a.t.m.a.n

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

In the handling of netlink sockets is are two small and (probably
always) non fatal glitches. Only together they could lead to page
faults. It is quite unlikely that it happens, but you cannot say
that it is impossible.

Best regards,
	Sven
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.11 (GNU/Linux)

iQIcBAEBCgAGBQJKE+oyAAoJEF2HCgfBJntGsv4P/ih/7xcrweNox8Kv8+OhoWBV
sZ8DcgRI3REqFtZMCNte/P/6ZGSx1hE95NytTo2R767PJF1ZjKX4B45NGf52rtAp
y+5++SGZ6vRPXPDhEF/1yg4i0ce29pgwZJVYj0qXZk5uLyJz2j4RtkPDGuCm4MCd
V7B1V+tVTLOGKLGnJAMEi49F61dRsYNt5tobQKwNxGt6tZVC7CCD7Ze2jA3c2b8r
dGixm+4vQGrV0EEmUS4JoJloOlz9tUI8+ld65urwj5Ep8IpSYBfrjQRbd9oaVtQe
6vRXnuAdZz9nam9MZRm4IqGa5DfNBkd0vXQryy85sxPSuIFti0FtiQI3bGYX8QnV
7ShajIU4JRo4k6jqZX6H+Z65DBahUFrbrfGqip7eOSLUANwtHy4S/0MD4Aw4FU2X
FF6AfsrdiBdT68eVBsxJdXInrAu5UwXBtIwNQm90l8JzT3TOo5pnDGiA7z0fVRGb
IJbTQKqkpZNvf0esyF7PhcEnjwefsP9gdXzuTcqlAday/mX3m73TmMismlM22Id+
eSLLU03Qi+fM3HfmMCk7OlMYXPw47qI+vMX8NGNqh/3MSqXZv8z9BXts89DXjMxs
jDOFX/qZCx0Z5c31DjCH7sxi3/R8rQTSE/AUMLVUrttuGwS/WzE4m/8W/Yu13gMi
ERz3CKiMqSEDin8EyL6a
=ECeu
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [B.A.T.M.A.N.] [PATCH 1/2] [batman] Reserve enough space for aligned netlink requests
  2009-05-20 11:33 [B.A.T.M.A.N.] Possible page faults at route.c Sven Eckelmann
@ 2009-05-20 11:33 ` Sven Eckelmann
  2009-05-20 11:33 ` [B.A.T.M.A.N.] [PATCH 2/2] [batman] Don't add size for netlink header twice in netlink request Sven Eckelmann
  1 sibling, 0 replies; 5+ messages in thread
From: Sven Eckelmann @ 2009-05-20 11:33 UTC (permalink / raw)
  To: b.a.t.m.a.n

We are currently telling the kernel that our netlink request has a
special aligned size due to the usage of NLMSG_LENGTH. We don't
provide enough space to send this data to the kernel and thus create
invalid reads by the kernel when calling the sendto. The alignment is
currently small enough that we cannot create page faults which makes
these invalid reads alone a nonfatal problem.
We should provide a buffer with the exact amount of data we write in
nlh.nlmsg_len or even more.

Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>
---
 batman/linux/route.c |  138 ++++++++++++++++++++++++++------------------------
 1 files changed, 72 insertions(+), 66 deletions(-)

diff --git a/batman/linux/route.c b/batman/linux/route.c
index 8fe6015..3ddce2e 100644
--- a/batman/linux/route.c
+++ b/batman/linux/route.c
@@ -182,11 +182,12 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	struct iovec iov;
 	struct msghdr msg;
 	struct nlmsghdr *nh;
-	struct {
+	struct req_s {
 		struct nlmsghdr nlh;
 		struct rtmsg rtm;
 		char buff[4 * (sizeof(struct rtattr) + 4)];
-	} req;
+	} *req;
+	char req_buf[NLMSG_LENGTH(sizeof(struct req_s))];
 
 	iov.iov_base = buf;
 	iov.iov_len  = sizeof(buf);
@@ -227,8 +228,9 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	}
 
 
+	req = (struct req_s*)req_buf;
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(&req, 0, sizeof(req));
+	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
@@ -242,39 +244,39 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	if (src_ip != 0)
 		len += sizeof(struct rtattr) + 4;
 
-	req.nlh.nlmsg_len = NLMSG_LENGTH(len);
-	req.nlh.nlmsg_pid = getpid();
-	req.rtm.rtm_family = AF_INET;
-	req.rtm.rtm_table = rt_table;
-	req.rtm.rtm_dst_len = netmask;
+	req->nlh.nlmsg_len = NLMSG_LENGTH(len);
+	req->nlh.nlmsg_pid = getpid();
+	req->rtm.rtm_family = AF_INET;
+	req->rtm.rtm_table = rt_table;
+	req->rtm.rtm_dst_len = netmask;
 
 	if (route_action == ROUTE_DEL) {
 
-		req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
-		req.nlh.nlmsg_type = RTM_DELROUTE;
-		req.rtm.rtm_scope = RT_SCOPE_NOWHERE;
+		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+		req->nlh.nlmsg_type = RTM_DELROUTE;
+		req->rtm.rtm_scope = RT_SCOPE_NOWHERE;
 
 	} else {
 
-		req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_APPEND;
-		req.nlh.nlmsg_type = RTM_NEWROUTE;
+		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_APPEND;
+		req->nlh.nlmsg_type = RTM_NEWROUTE;
 
 		if (route_type == ROUTE_TYPE_UNICAST && my_router == 0 && src_ip != 0)
-			req.rtm.rtm_scope = RT_SCOPE_LINK;
+			req->rtm.rtm_scope = RT_SCOPE_LINK;
 		else
-			req.rtm.rtm_scope = RT_SCOPE_UNIVERSE;
+			req->rtm.rtm_scope = RT_SCOPE_UNIVERSE;
 
-		req.rtm.rtm_protocol = RTPROT_STATIC; /* may be changed to some batman specific value - see <linux/rtnetlink.h> */
+		req->rtm.rtm_protocol = RTPROT_STATIC; /* may be changed to some batman specific value - see <linux/rtnetlink.h> */
 
 		switch(route_type) {
 		case ROUTE_TYPE_UNICAST:
-			req.rtm.rtm_type = RTN_UNICAST;
+			req->rtm.rtm_type = RTN_UNICAST;
 			break;
 		case ROUTE_TYPE_THROW:
-			req.rtm.rtm_type = RTN_THROW;
+			req->rtm.rtm_type = RTN_THROW;
 			break;
 		case ROUTE_TYPE_UNREACHABLE:
-			req.rtm.rtm_type = RTN_UNREACHABLE;
+			req->rtm.rtm_type = RTN_UNREACHABLE;
 			break;
 		default:
 			debug_output(0, "Error - unknown route type (add_del_route): %i\n", route_type);
@@ -282,28 +284,28 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 		}
 	}
 
-	rta = (struct rtattr *)req.buff;
+	rta = (struct rtattr *)req->buff;
 	rta->rta_type = RTA_DST;
 	rta->rta_len = sizeof(struct rtattr) + 4;
-	memcpy(((char *)&req.buff) + sizeof(struct rtattr), (char *)&dest, 4);
+	memcpy(((char *)req->buff) + sizeof(struct rtattr), (char *)&dest, 4);
 
 	if (route_type == ROUTE_TYPE_UNICAST) {
 
-		rta = (struct rtattr *)(req.buff + sizeof(struct rtattr) + 4);
+		rta = (struct rtattr *)(req->buff + sizeof(struct rtattr) + 4);
 		rta->rta_type = RTA_GATEWAY;
 		rta->rta_len = sizeof(struct rtattr) + 4;
-		memcpy(((char *)&req.buff) + 2 * sizeof(struct rtattr) + 4, (char *)&my_router, 4);
+		memcpy(((char *)req->buff) + 2 * sizeof(struct rtattr) + 4, (char *)&my_router, 4);
 
-		rta = (struct rtattr *)(req.buff + 2 * sizeof(struct rtattr) + 8);
+		rta = (struct rtattr *)(req->buff + 2 * sizeof(struct rtattr) + 8);
 		rta->rta_type = RTA_OIF;
 		rta->rta_len = sizeof(struct rtattr) + 4;
-		memcpy(((char *)&req.buff) + 3 * sizeof(struct rtattr) + 8, (char *)&ifi, 4);
+		memcpy(((char *)req->buff) + 3 * sizeof(struct rtattr) + 8, (char *)&ifi, 4);
 
 		if (src_ip != 0) {
-			rta = (struct rtattr *)(req.buff + 3 * sizeof(struct rtattr) + 12);
+			rta = (struct rtattr *)(req->buff + 3 * sizeof(struct rtattr) + 12);
 			rta->rta_type = RTA_PREFSRC;
 			rta->rta_len = sizeof(struct rtattr) + 4;
-			memcpy(((char *)&req.buff) + 4 * sizeof(struct rtattr) + 12, (char *)&src_ip, 4);
+			memcpy(((char *)req->buff) + 4 * sizeof(struct rtattr) + 12, (char *)&src_ip, 4);
 		}
 
 	}
@@ -315,7 +317,7 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 
 	}
 
-	if (sendto(netlink_sock, &req, req.nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
+	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
 
 		debug_output(0, "Error - can't send message to kernel via netlink socket for routing table manipulation: %s\n", strerror(errno));
 		close(netlink_sock);
@@ -364,11 +366,12 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	struct iovec iov;
 	struct msghdr msg;
 	struct nlmsghdr *nh;
-	struct {
+	struct req_s {
 		struct nlmsghdr nlh;
 		struct rtmsg rtm;
 		char buff[2 * (sizeof(struct rtattr) + 4)];
-	} req;
+	} *req;
+	char req_buf[NLMSG_LENGTH(sizeof(struct req_s))];
 
 	iov.iov_base = buf;
 	iov.iov_len  = sizeof(buf);
@@ -382,8 +385,9 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	}
 
 
+	req = (struct req_s*)req_buf;
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(&req, 0, sizeof(req));
+	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
@@ -393,53 +397,53 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	if (prio != 0)
 		len += sizeof(struct rtattr) + 4;
 
-	req.nlh.nlmsg_len = NLMSG_LENGTH(len);
-	req.nlh.nlmsg_pid = getpid();
-	req.rtm.rtm_family = AF_INET;
-	req.rtm.rtm_table = rt_table;
+	req->nlh.nlmsg_len = NLMSG_LENGTH(len);
+	req->nlh.nlmsg_pid = getpid();
+	req->rtm.rtm_family = AF_INET;
+	req->rtm.rtm_table = rt_table;
 
 
 	if (rule_action == RULE_DEL) {
 
-		req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
-		req.nlh.nlmsg_type = RTM_DELRULE;
-		req.rtm.rtm_scope = RT_SCOPE_NOWHERE;
+		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+		req->nlh.nlmsg_type = RTM_DELRULE;
+		req->rtm.rtm_scope = RT_SCOPE_NOWHERE;
 
 	} else {
 
-		req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
-		req.nlh.nlmsg_type = RTM_NEWRULE;
-		req.rtm.rtm_scope = RT_SCOPE_UNIVERSE;
-		req.rtm.rtm_protocol = RTPROT_STATIC;
-		req.rtm.rtm_type = RTN_UNICAST;
+		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
+		req->nlh.nlmsg_type = RTM_NEWRULE;
+		req->rtm.rtm_scope = RT_SCOPE_UNIVERSE;
+		req->rtm.rtm_protocol = RTPROT_STATIC;
+		req->rtm.rtm_type = RTN_UNICAST;
 
 	}
 
 	switch(rule_type) {
 	case RULE_TYPE_SRC:
-		rta = (struct rtattr *)req.buff;
+		rta = (struct rtattr *)req->buff;
 		rta->rta_type = RTA_SRC;
-		req.rtm.rtm_src_len = netmask;
+		req->rtm.rtm_src_len = netmask;
 		rta->rta_len = sizeof(struct rtattr) + 4;
-		memcpy(((char *)&req.buff) + sizeof(struct rtattr), (char *)&network, 4);
+		memcpy(((char *)req->buff) + sizeof(struct rtattr), (char *)&network, 4);
 		break;
 	case RULE_TYPE_DST:
-		rta = (struct rtattr *)req.buff;
+		rta = (struct rtattr *)req->buff;
 		rta->rta_type = RTA_DST;
-		req.rtm.rtm_dst_len = netmask;
+		req->rtm.rtm_dst_len = netmask;
 		rta->rta_len = sizeof(struct rtattr) + 4;
-		memcpy(((char *)&req.buff) + sizeof(struct rtattr), (char *)&network, 4);
+		memcpy(((char *)req->buff) + sizeof(struct rtattr), (char *)&network, 4);
 		break;
 	case RULE_TYPE_IIF:
-		rta = (struct rtattr *)req.buff;
+		rta = (struct rtattr *)req->buff;
 		rta->rta_len = sizeof(struct rtattr) + 4;
 
 		if (rule_action == RULE_DEL) {
 			rta->rta_type = RTA_SRC;
-			memcpy(((char *)&req.buff) + sizeof(struct rtattr), (char *)&network, 4);
+			memcpy(((char *)req->buff) + sizeof(struct rtattr), (char *)&network, 4);
 		} else {
 			rta->rta_type = RTA_IIF;
-			memcpy(((char *)&req.buff) + sizeof(struct rtattr), iif, 4);
+			memcpy(((char *)req->buff) + sizeof(struct rtattr), iif, 4);
 		}
 		break;
 	default:
@@ -448,10 +452,10 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	}
 
 	if (prio != 0) {
-		rta = (struct rtattr *)(req.buff + sizeof(struct rtattr) + 4);
+		rta = (struct rtattr *)(req->buff + sizeof(struct rtattr) + 4);
 		rta->rta_type = RTA_PRIORITY;
 		rta->rta_len = sizeof(struct rtattr) + 4;
-		memcpy(((char *)&req.buff) + 2 * sizeof(struct rtattr) + 4, (char *)&prio, 4);
+		memcpy(((char *)req->buff) + 2 * sizeof(struct rtattr) + 4, (char *)&prio, 4);
 	}
 
 
@@ -463,7 +467,7 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	}
 
 
-	if (sendto(netlink_sock, &req, req.nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0)  {
+	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0)  {
 
 		debug_output( 0, "Error - can't send message to kernel via netlink socket for routing rule manipulation: %s\n", strerror(errno));
 		close(netlink_sock);
@@ -628,29 +632,31 @@ int flush_routes_rules(int8_t is_rule)
 	struct msghdr msg;
 	struct nlmsghdr *nh;
 	struct rtmsg *rtm;
-	struct {
+	struct req_s {
 		struct nlmsghdr nlh;
 		struct rtmsg rtm;
-	} req;
+	} *req;
+	char req_buf[NLMSG_LENGTH(sizeof(struct req_s))];
+
 	struct rtattr *rtap;
 
 	iov.iov_base = buf;
 	iov.iov_len  = sizeof(buf);
 
-
+	req = (struct req_s*)req_buf;
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(&req, 0, sizeof(req));
+	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
 
-	req.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(req));
-	req.nlh.nlmsg_pid = getpid();
-	req.rtm.rtm_family = AF_INET;
+	req->nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct req_s));
+	req->nlh.nlmsg_pid = getpid();
+	req->rtm.rtm_family = AF_INET;
 
-	req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
-	req.nlh.nlmsg_type = (is_rule ? RTM_GETRULE : RTM_GETROUTE);
-	req.rtm.rtm_scope = RTN_UNICAST;
+	req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+	req->nlh.nlmsg_type = (is_rule ? RTM_GETRULE : RTM_GETROUTE);
+	req->rtm.rtm_scope = RTN_UNICAST;
 
 	if ((netlink_sock = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE)) < 0) {
 
@@ -660,7 +666,7 @@ int flush_routes_rules(int8_t is_rule)
 	}
 
 
-	if (sendto(netlink_sock, &req, req.nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
+	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
 
 		debug_output(0, "Error - can't send message to kernel via netlink socket for flushing the routing table: %s\n", strerror(errno));
 		close(netlink_sock);
-- 
1.6.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [B.A.T.M.A.N.] [PATCH 2/2] [batman] Don't add size for netlink header twice in netlink request
  2009-05-20 11:33 [B.A.T.M.A.N.] Possible page faults at route.c Sven Eckelmann
  2009-05-20 11:33 ` [B.A.T.M.A.N.] [PATCH 1/2] [batman] Reserve enough space for aligned netlink requests Sven Eckelmann
@ 2009-05-20 11:33 ` Sven Eckelmann
  2009-05-20 16:25   ` [B.A.T.M.A.N.] [PATCH2/2v2] " Sven Eckelmann
  1 sibling, 1 reply; 5+ messages in thread
From: Sven Eckelmann @ 2009-05-20 11:33 UTC (permalink / raw)
  To: b.a.t.m.a.n

The parameter len of NLMSG_LENGTH is only the size of the payload of
the request. If we use this function to calculate the size of the buffer
we send to the kernel we would add (sizeof(nlmsghdr)+padding) and the
size of our own structure which also has a nlmsghdr included. We must
split the req structure into a header part and the payload to calculate
the correct size of our buffer and having the payload always start at
the correct address.

Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>
---
 batman/linux/route.c |   59 ++++++++++++++++++++++++++-----------------------
 1 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/batman/linux/route.c b/batman/linux/route.c
index 3ddce2e..4be0179 100644
--- a/batman/linux/route.c
+++ b/batman/linux/route.c
@@ -182,8 +182,8 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	struct iovec iov;
 	struct msghdr msg;
 	struct nlmsghdr *nh;
+	struct nlmsghdr *nlh;
 	struct req_s {
-		struct nlmsghdr nlh;
 		struct rtmsg rtm;
 		char buff[4 * (sizeof(struct rtattr) + 4)];
 	} *req;
@@ -228,9 +228,10 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	}
 
 
-	req = (struct req_s*)req_buf;
+	nlh = (struct nlmsghdr *)req_buf;
+	req = (struct req_s*)NLMSG_DATA(req_buf);
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
+	memset(req_buf, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
@@ -244,22 +245,22 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	if (src_ip != 0)
 		len += sizeof(struct rtattr) + 4;
 
-	req->nlh.nlmsg_len = NLMSG_LENGTH(len);
-	req->nlh.nlmsg_pid = getpid();
+	nlh->nlmsg_len = NLMSG_LENGTH(len);
+	nlh->nlmsg_pid = getpid();
 	req->rtm.rtm_family = AF_INET;
 	req->rtm.rtm_table = rt_table;
 	req->rtm.rtm_dst_len = netmask;
 
 	if (route_action == ROUTE_DEL) {
 
-		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
-		req->nlh.nlmsg_type = RTM_DELROUTE;
+		nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+		nlh->nlmsg_type = RTM_DELROUTE;
 		req->rtm.rtm_scope = RT_SCOPE_NOWHERE;
 
 	} else {
 
-		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_APPEND;
-		req->nlh.nlmsg_type = RTM_NEWROUTE;
+		nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_APPEND;
+		nlh->nlmsg_type = RTM_NEWROUTE;
 
 		if (route_type == ROUTE_TYPE_UNICAST && my_router == 0 && src_ip != 0)
 			req->rtm.rtm_scope = RT_SCOPE_LINK;
@@ -317,7 +318,7 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 
 	}
 
-	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
+	if (sendto(netlink_sock, req_buf, nlh->nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
 
 		debug_output(0, "Error - can't send message to kernel via netlink socket for routing table manipulation: %s\n", strerror(errno));
 		close(netlink_sock);
@@ -366,8 +367,8 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	struct iovec iov;
 	struct msghdr msg;
 	struct nlmsghdr *nh;
+	struct nlmsghdr *nlh;
 	struct req_s {
-		struct nlmsghdr nlh;
 		struct rtmsg rtm;
 		char buff[2 * (sizeof(struct rtattr) + 4)];
 	} *req;
@@ -385,9 +386,10 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	}
 
 
-	req = (struct req_s*)req_buf;
+	nlh = (struct nlmsghdr *)req_buf;
+	req = (struct req_s*)NLMSG_DATA(req_buf);
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
+	memset(req_buf, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
@@ -397,22 +399,22 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	if (prio != 0)
 		len += sizeof(struct rtattr) + 4;
 
-	req->nlh.nlmsg_len = NLMSG_LENGTH(len);
-	req->nlh.nlmsg_pid = getpid();
+	nlh->nlmsg_len = NLMSG_LENGTH(len);
+	nlh->nlmsg_pid = getpid();
 	req->rtm.rtm_family = AF_INET;
 	req->rtm.rtm_table = rt_table;
 
 
 	if (rule_action == RULE_DEL) {
 
-		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
-		req->nlh.nlmsg_type = RTM_DELRULE;
+		nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+		nlh->nlmsg_type = RTM_DELRULE;
 		req->rtm.rtm_scope = RT_SCOPE_NOWHERE;
 
 	} else {
 
-		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
-		req->nlh.nlmsg_type = RTM_NEWRULE;
+		nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
+		nlh->nlmsg_type = RTM_NEWRULE;
 		req->rtm.rtm_scope = RT_SCOPE_UNIVERSE;
 		req->rtm.rtm_protocol = RTPROT_STATIC;
 		req->rtm.rtm_type = RTN_UNICAST;
@@ -467,7 +469,7 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	}
 
 
-	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0)  {
+	if (sendto(netlink_sock, req_buf, nlh->nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0)  {
 
 		debug_output( 0, "Error - can't send message to kernel via netlink socket for routing rule manipulation: %s\n", strerror(errno));
 		close(netlink_sock);
@@ -632,8 +634,8 @@ int flush_routes_rules(int8_t is_rule)
 	struct msghdr msg;
 	struct nlmsghdr *nh;
 	struct rtmsg *rtm;
+	struct nlmsghdr *nlh;
 	struct req_s {
-		struct nlmsghdr nlh;
 		struct rtmsg rtm;
 	} *req;
 	char req_buf[NLMSG_LENGTH(sizeof(struct req_s))];
@@ -643,19 +645,20 @@ int flush_routes_rules(int8_t is_rule)
 	iov.iov_base = buf;
 	iov.iov_len  = sizeof(buf);
 
-	req = (struct req_s*)req_buf;
+	nlh = (struct nlmsghdr *)req_buf;
+	req = (struct req_s*)NLMSG_DATA(req_buf);
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
+	memset(req_buf, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
 
-	req->nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct req_s));
-	req->nlh.nlmsg_pid = getpid();
+	nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct req_s));
+	nlh->nlmsg_pid = getpid();
 	req->rtm.rtm_family = AF_INET;
 
-	req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
-	req->nlh.nlmsg_type = (is_rule ? RTM_GETRULE : RTM_GETROUTE);
+	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+	nlh->nlmsg_type = (is_rule ? RTM_GETRULE : RTM_GETROUTE);
 	req->rtm.rtm_scope = RTN_UNICAST;
 
 	if ((netlink_sock = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE)) < 0) {
@@ -666,7 +669,7 @@ int flush_routes_rules(int8_t is_rule)
 	}
 
 
-	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
+	if (sendto(netlink_sock, req_buf, nlh->nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
 
 		debug_output(0, "Error - can't send message to kernel via netlink socket for flushing the routing table: %s\n", strerror(errno));
 		close(netlink_sock);
-- 
1.6.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [B.A.T.M.A.N.] [PATCH2/2v2] [batman] Don't add size for netlink header twice in netlink request
  2009-05-20 11:33 ` [B.A.T.M.A.N.] [PATCH 2/2] [batman] Don't add size for netlink header twice in netlink request Sven Eckelmann
@ 2009-05-20 16:25   ` Sven Eckelmann
  2009-05-20 17:30     ` Marek Lindner
  0 siblings, 1 reply; 5+ messages in thread
From: Sven Eckelmann @ 2009-05-20 16:25 UTC (permalink / raw)
  To: b.a.t.m.a.n

The parameter len of NLMSG_LENGTH is only the size of the payload of
the request. If we use this function to calculate the size of the buffer
we send to the kernel we would add (sizeof(nlmsghdr)+padding) and the
size of our own structure which also has a nlmsghdr included. We must
split the req structure into a header part and the payload to calculate
the correct size of our buffer and having the payload always start at
the correct address.

Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>
---
 batman/linux/route.c |   56 +++++++++++++++++++++++++-------------------------
 1 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/batman/linux/route.c b/batman/linux/route.c
index 3ddce2e..99c295f 100644
--- a/batman/linux/route.c
+++ b/batman/linux/route.c
@@ -183,7 +183,6 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	struct msghdr msg;
 	struct nlmsghdr *nh;
 	struct req_s {
-		struct nlmsghdr nlh;
 		struct rtmsg rtm;
 		char buff[4 * (sizeof(struct rtattr) + 4)];
 	} *req;
@@ -228,9 +227,10 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	}
 
 
-	req = (struct req_s*)req_buf;
+	nh = (struct nlmsghdr *)req_buf;
+	req = (struct req_s*)NLMSG_DATA(req_buf);
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
+	memset(req_buf, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
@@ -244,22 +244,22 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 	if (src_ip != 0)
 		len += sizeof(struct rtattr) + 4;
 
-	req->nlh.nlmsg_len = NLMSG_LENGTH(len);
-	req->nlh.nlmsg_pid = getpid();
+	nh->nlmsg_len = NLMSG_LENGTH(len);
+	nh->nlmsg_pid = getpid();
 	req->rtm.rtm_family = AF_INET;
 	req->rtm.rtm_table = rt_table;
 	req->rtm.rtm_dst_len = netmask;
 
 	if (route_action == ROUTE_DEL) {
 
-		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
-		req->nlh.nlmsg_type = RTM_DELROUTE;
+		nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+		nh->nlmsg_type = RTM_DELROUTE;
 		req->rtm.rtm_scope = RT_SCOPE_NOWHERE;
 
 	} else {
 
-		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_APPEND;
-		req->nlh.nlmsg_type = RTM_NEWROUTE;
+		nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_APPEND;
+		nh->nlmsg_type = RTM_NEWROUTE;
 
 		if (route_type == ROUTE_TYPE_UNICAST && my_router == 0 && src_ip != 0)
 			req->rtm.rtm_scope = RT_SCOPE_LINK;
@@ -317,7 +317,7 @@ void add_del_route(uint32_t dest, uint8_t netmask, uint32_t router, uint32_t src
 
 	}
 
-	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
+	if (sendto(netlink_sock, req_buf, nh->nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
 
 		debug_output(0, "Error - can't send message to kernel via netlink socket for routing table manipulation: %s\n", strerror(errno));
 		close(netlink_sock);
@@ -367,7 +367,6 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	struct msghdr msg;
 	struct nlmsghdr *nh;
 	struct req_s {
-		struct nlmsghdr nlh;
 		struct rtmsg rtm;
 		char buff[2 * (sizeof(struct rtattr) + 4)];
 	} *req;
@@ -385,9 +384,10 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	}
 
 
-	req = (struct req_s*)req_buf;
+	nh = (struct nlmsghdr *)req_buf;
+	req = (struct req_s*)NLMSG_DATA(req_buf);
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
+	memset(req_buf, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
@@ -397,22 +397,22 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	if (prio != 0)
 		len += sizeof(struct rtattr) + 4;
 
-	req->nlh.nlmsg_len = NLMSG_LENGTH(len);
-	req->nlh.nlmsg_pid = getpid();
+	nh->nlmsg_len = NLMSG_LENGTH(len);
+	nh->nlmsg_pid = getpid();
 	req->rtm.rtm_family = AF_INET;
 	req->rtm.rtm_table = rt_table;
 
 
 	if (rule_action == RULE_DEL) {
 
-		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
-		req->nlh.nlmsg_type = RTM_DELRULE;
+		nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+		nh->nlmsg_type = RTM_DELRULE;
 		req->rtm.rtm_scope = RT_SCOPE_NOWHERE;
 
 	} else {
 
-		req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
-		req->nlh.nlmsg_type = RTM_NEWRULE;
+		nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
+		nh->nlmsg_type = RTM_NEWRULE;
 		req->rtm.rtm_scope = RT_SCOPE_UNIVERSE;
 		req->rtm.rtm_protocol = RTPROT_STATIC;
 		req->rtm.rtm_type = RTN_UNICAST;
@@ -467,7 +467,7 @@ void add_del_rule(uint32_t network, uint8_t netmask, int8_t rt_table, uint32_t p
 	}
 
 
-	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0)  {
+	if (sendto(netlink_sock, req_buf, nh->nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0)  {
 
 		debug_output( 0, "Error - can't send message to kernel via netlink socket for routing rule manipulation: %s\n", strerror(errno));
 		close(netlink_sock);
@@ -633,7 +633,6 @@ int flush_routes_rules(int8_t is_rule)
 	struct nlmsghdr *nh;
 	struct rtmsg *rtm;
 	struct req_s {
-		struct nlmsghdr nlh;
 		struct rtmsg rtm;
 	} *req;
 	char req_buf[NLMSG_LENGTH(sizeof(struct req_s))];
@@ -643,19 +642,20 @@ int flush_routes_rules(int8_t is_rule)
 	iov.iov_base = buf;
 	iov.iov_len  = sizeof(buf);
 
-	req = (struct req_s*)req_buf;
+	nh = (struct nlmsghdr *)req_buf;
+	req = (struct req_s*)NLMSG_DATA(req_buf);
 	memset(&nladdr, 0, sizeof(struct sockaddr_nl));
-	memset(req, 0, NLMSG_LENGTH(sizeof(struct req_s)));
+	memset(req_buf, 0, NLMSG_LENGTH(sizeof(struct req_s)));
 	memset(&msg, 0, sizeof(struct msghdr));
 
 	nladdr.nl_family = AF_NETLINK;
 
-	req->nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct req_s));
-	req->nlh.nlmsg_pid = getpid();
+	nh->nlmsg_len = NLMSG_LENGTH(sizeof(struct req_s));
+	nh->nlmsg_pid = getpid();
 	req->rtm.rtm_family = AF_INET;
 
-	req->nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
-	req->nlh.nlmsg_type = (is_rule ? RTM_GETRULE : RTM_GETROUTE);
+	nh->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+	nh->nlmsg_type = (is_rule ? RTM_GETRULE : RTM_GETROUTE);
 	req->rtm.rtm_scope = RTN_UNICAST;
 
 	if ((netlink_sock = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE)) < 0) {
@@ -666,7 +666,7 @@ int flush_routes_rules(int8_t is_rule)
 	}
 
 
-	if (sendto(netlink_sock, req, req->nlh.nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
+	if (sendto(netlink_sock, req_buf, nh->nlmsg_len, 0, (struct sockaddr *)&nladdr, sizeof(struct sockaddr_nl)) < 0) {
 
 		debug_output(0, "Error - can't send message to kernel via netlink socket for flushing the routing table: %s\n", strerror(errno));
 		close(netlink_sock);
-- 
1.6.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH2/2v2] [batman] Don't add size for netlink header twice in netlink request
  2009-05-20 16:25   ` [B.A.T.M.A.N.] [PATCH2/2v2] " Sven Eckelmann
@ 2009-05-20 17:30     ` Marek Lindner
  0 siblings, 0 replies; 5+ messages in thread
From: Marek Lindner @ 2009-05-20 17:30 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Thursday 21 May 2009 00:25:56 Sven Eckelmann wrote:
> The parameter len of NLMSG_LENGTH is only the size of the payload of
> the request. If we use this function to calculate the size of the buffer
> we send to the kernel we would add (sizeof(nlmsghdr)+padding) and the
> size of our own structure which also has a nlmsghdr included. We must
> split the req structure into a header part and the payload to calculate
> the correct size of our buffer and having the payload always start at
> the correct address.
>
> Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>

Great findings - the patches are in now.  :-)

Regards,
Marek


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2009-05-20 17:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-20 11:33 [B.A.T.M.A.N.] Possible page faults at route.c Sven Eckelmann
2009-05-20 11:33 ` [B.A.T.M.A.N.] [PATCH 1/2] [batman] Reserve enough space for aligned netlink requests Sven Eckelmann
2009-05-20 11:33 ` [B.A.T.M.A.N.] [PATCH 2/2] [batman] Don't add size for netlink header twice in netlink request Sven Eckelmann
2009-05-20 16:25   ` [B.A.T.M.A.N.] [PATCH2/2v2] " Sven Eckelmann
2009-05-20 17:30     ` Marek Lindner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).