From b4919a5f95c09992f66d4b7cbe392c33731a5cec Mon Sep 17 00:00:00 2001 From: Olaf Hering Date: Thu, 1 Aug 2013 14:34:26 +0200 Subject: Tools: hv: fix send/recv buffer allocation hv_kvp_daemon fails to start in current openSuSE 13.1 snapshots because the kvp_send_buffer is too small to hold cn_msg+hv_kvp_msg, the very first sendmsg returns with EFAULT. In addition it fixes the Network info tab in Windows Server 2012R2 in SLES11. Adjust the code in kvp and vss daemon to allocate the needed buffers at runtime. To keep the code simple, the buffer_len includes also the nlmsghdr, although only the recv_buffer needs this extra space. Signed-off-by: Olaf Hering Signed-off-by: K. Y. Srinivasan Signed-off-by: Greg Kroah-Hartman --- tools/hv/hv_kvp_daemon.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'tools/hv/hv_kvp_daemon.c') diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index 07819bfa7dba..657c1d27e02e 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -79,8 +79,6 @@ enum { DNS }; -static char kvp_send_buffer[4096]; -static char kvp_recv_buffer[4096 * 2]; static struct sockaddr_nl addr; static int in_hand_shake = 1; @@ -1437,10 +1435,21 @@ int main(void) int pool; char *if_name; struct hv_kvp_ipaddr_value *kvp_ip_val; + char *kvp_send_buffer; + char *kvp_recv_buffer; + size_t kvp_recv_buffer_len; daemon(1, 0); openlog("KVP", 0, LOG_USER); syslog(LOG_INFO, "KVP starting; pid is:%d", getpid()); + + kvp_recv_buffer_len = NLMSG_HDRLEN + sizeof(struct cn_msg) + sizeof(struct hv_kvp_msg); + kvp_send_buffer = calloc(1, kvp_recv_buffer_len); + kvp_recv_buffer = calloc(1, kvp_recv_buffer_len); + if (!(kvp_send_buffer && kvp_recv_buffer)) { + syslog(LOG_ERR, "Failed to allocate netlink buffers"); + exit(EXIT_FAILURE); + } /* * Retrieve OS release information. */ @@ -1514,7 +1523,7 @@ int main(void) continue; } - len = recvfrom(fd, kvp_recv_buffer, sizeof(kvp_recv_buffer), 0, + len = recvfrom(fd, kvp_recv_buffer, kvp_recv_buffer_len, 0, addr_p, &addr_l); if (len < 0) { -- cgit v1.2.1 From 00663d73e39a4aec0c310bb5fc1c2c8dfccf1319 Mon Sep 17 00:00:00 2001 From: Olaf Hering Date: Thu, 1 Aug 2013 14:43:12 +0200 Subject: Tools: hv: check return value of daemon to fix compiler warning. hv_kvp_daemon.c: In function 'main': hv_kvp_daemon.c:1441:8: warning: ignoring return value of 'daemon', declared with attribute warn_unused_result [-Wunused-result] Signed-off-by: Olaf Hering Signed-off-by: K. Y. Srinivasan Signed-off-by: Greg Kroah-Hartman --- tools/hv/hv_kvp_daemon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tools/hv/hv_kvp_daemon.c') diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index 657c1d27e02e..418ac5548f98 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -1439,7 +1439,8 @@ int main(void) char *kvp_recv_buffer; size_t kvp_recv_buffer_len; - daemon(1, 0); + if (daemon(1, 0)) + return 1; openlog("KVP", 0, LOG_USER); syslog(LOG_INFO, "KVP starting; pid is:%d", getpid()); -- cgit v1.2.1 From 57969af029b073a99ce27c2170cbcac5bd557606 Mon Sep 17 00:00:00 2001 From: Olaf Hering Date: Sun, 4 Aug 2013 16:41:24 +0200 Subject: Tools: hv: in kvp_set_ip_info free mac_addr right after usage ... to simplify error path in upcoming changes. Signed-off-by: Olaf Hering Signed-off-by: K. Y. Srinivasan Signed-off-by: Greg Kroah-Hartman --- tools/hv/hv_kvp_daemon.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'tools/hv/hv_kvp_daemon.c') diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index 418ac5548f98..ba075e514422 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -1299,6 +1299,7 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) } error = kvp_write_file(file, "HWADDR", "", mac_addr); + free(mac_addr); if (error) goto setval_error; @@ -1344,7 +1345,6 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) goto setval_error; setval_done: - free(mac_addr); fclose(file); /* @@ -1358,7 +1358,6 @@ setval_done: setval_error: syslog(LOG_ERR, "Failed to write config file"); - free(mac_addr); fclose(file); return error; } -- cgit v1.2.1 From d3b688c6622334e8460e808755d7d9c4a78c3ae5 Mon Sep 17 00:00:00 2001 From: Olaf Hering Date: Sun, 4 Aug 2013 16:40:44 +0200 Subject: Tools: hv: check return value of system in hv_kvp_daemon hv_kvp_daemon.c: In function 'main': hv_kvp_daemon.c:1441:8: warning: ignoring return value of 'daemon', declared with attribute warn_unused_result [-Wunused-result] Signed-off-by: Olaf Hering Signed-off-by: K. Y. Srinivasan Signed-off-by: Greg Kroah-Hartman --- tools/hv/hv_kvp_daemon.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'tools/hv/hv_kvp_daemon.c') diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index ba075e514422..b96eccce48e3 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -1353,7 +1353,11 @@ setval_done: */ snprintf(cmd, sizeof(cmd), "%s %s", "hv_set_ifconfig", if_file); - system(cmd); + if (system(cmd)) { + syslog(LOG_ERR, "Failed to execute cmd '%s'; error: %d %s", + cmd, errno, strerror(errno)); + return HV_E_FAIL; + } return 0; setval_error: -- cgit v1.2.1 From 2bc41ea3b3fd4c2f2473ec84f4ee3ef5ff21e49b Mon Sep 17 00:00:00 2001 From: Olaf Hering Date: Wed, 7 Aug 2013 15:07:21 +0200 Subject: Tools: hv: correct payload size in netlink_send netlink_send is supposed to send just the cn_msg+hv_kvp_msg via netlink. Currently it sets an incorrect iovec size, as reported by valgrind. In the case of registering with the kernel the allocated buffer is large enough to hold nlmsghdr+cn_msg+hv_kvp_msg, no overrun happens. In the case of responding to the kernel the cn_msg is located in the middle of recv_buffer, after the nlmsghdr. Currently the code in netlink_send adds also the size of nlmsghdr to the payload. But nlmsghdr is a separate iovec. This leads to an (harmless) out-of-bounds access when the kernel processes the iovec. Correct the iovec size of the cn_msg to be just cn_msg + its payload. Signed-off-by: Olaf Hering Signed-off-by: K. Y. Srinivasan Signed-off-by: Greg Kroah-Hartman --- tools/hv/hv_kvp_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/hv/hv_kvp_daemon.c') diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index b96eccce48e3..dca06a26ee2a 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -1398,7 +1398,7 @@ netlink_send(int fd, struct cn_msg *msg) char buffer[64]; struct iovec iov[2]; - size = NLMSG_SPACE(sizeof(struct cn_msg) + msg->len); + size = sizeof(struct cn_msg) + msg->len; nlh = (struct nlmsghdr *)buffer; nlh->nlmsg_seq = 0; -- cgit v1.2.1 From b4fb0ca26055bb39b18a1427eea633877a3dcc80 Mon Sep 17 00:00:00 2001 From: Olaf Hering Date: Wed, 7 Aug 2013 15:45:12 +0200 Subject: Tools: hv: use full nlmsghdr in netlink_send There is no need to have a nlmsghdr pointer to another temporary buffer. Instead use a full struct nlmsghdr. Signed-off-by: Olaf Hering Signed-off-by: K. Y. Srinivasan Signed-off-by: Greg Kroah-Hartman --- tools/hv/hv_kvp_daemon.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) (limited to 'tools/hv/hv_kvp_daemon.c') diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index dca06a26ee2a..8fd9ec66121c 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -1392,23 +1392,18 @@ kvp_get_domain_name(char *buffer, int length) static int netlink_send(int fd, struct cn_msg *msg) { - struct nlmsghdr *nlh; + struct nlmsghdr nlh = { .nlmsg_type = NLMSG_DONE }; unsigned int size; struct msghdr message; - char buffer[64]; struct iovec iov[2]; size = sizeof(struct cn_msg) + msg->len; - nlh = (struct nlmsghdr *)buffer; - nlh->nlmsg_seq = 0; - nlh->nlmsg_pid = getpid(); - nlh->nlmsg_type = NLMSG_DONE; - nlh->nlmsg_len = NLMSG_LENGTH(size - sizeof(*nlh)); - nlh->nlmsg_flags = 0; + nlh.nlmsg_pid = getpid(); + nlh.nlmsg_len = NLMSG_LENGTH(size); - iov[0].iov_base = nlh; - iov[0].iov_len = sizeof(*nlh); + iov[0].iov_base = &nlh; + iov[0].iov_len = sizeof(nlh); iov[1].iov_base = msg; iov[1].iov_len = size; -- cgit v1.2.1