From 3c86e0ea5d2c1a528d91d76d27c8ae83c02cf2b9 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Sat, 30 Jan 2016 23:50:54 +0000 Subject: [PATCH] Robustify netlink response parsers * socketutils.c (inet_parse_response, unix_parse_response): Change return type from bool to int, return -1 on all parse errors except inode mismatch. (receive_responses): Stop on the first nlmsg_type that is not SOCK_DIAG_BY_FAMILY, also stop when the parser returns -1. * tests/netlink_inet_diag.c (check_responses): Stop on short messages, on first nlmsg_type that is not SOCK_DIAG_BY_FAMILY, print more verbose diagnostics for NLMSG_ERROR. * tests/netlink_unix_diag.c (check_responses): Likewise. --- socketutils.c | 69 ++++++++++++++++++++------------------- tests/netlink_inet_diag.c | 27 +++++++++++---- tests/netlink_unix_diag.c | 27 +++++++++++---- 3 files changed, 75 insertions(+), 48 deletions(-) diff --git a/socketutils.c b/socketutils.c index aa821639..6fbb8cfe 100644 --- a/socketutils.c +++ b/socketutils.c @@ -1,6 +1,6 @@ /* * Copyright (c) 2014 Zubin Mithra - * Copyright (c) 2014-2015 Dmitry V. Levin + * Copyright (c) 2014-2016 Dmitry V. Levin * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -71,7 +71,7 @@ inet_send_query(const int fd, const int family, const int proto) .iov_len = sizeof(req) }; struct msghdr msg = { - .msg_name = (void*)&nladdr, + .msg_name = (void *) &nladdr, .msg_namelen = sizeof(nladdr), .msg_iov = &iov, .msg_iovlen = 1 @@ -87,18 +87,18 @@ inet_send_query(const int fd, const int family, const int proto) } } -static bool -inet_parse_response(const char *proto_name, const void *data, int data_len, - const unsigned long inode) +static int +inet_parse_response(const char *proto_name, const void *data, + const int data_len, const unsigned long inode) { const struct inet_diag_msg *diag_msg = data; static const char zero_addr[sizeof(struct in6_addr)]; socklen_t addr_size, text_size; if (data_len < (int) NLMSG_LENGTH(sizeof(*diag_msg))) - return false; + return -1; if (diag_msg->idiag_inode != inode) - return false; + return 0; switch(diag_msg->idiag_family) { case AF_INET: @@ -110,14 +110,14 @@ inet_parse_response(const char *proto_name, const void *data, int data_len, text_size = INET6_ADDRSTRLEN; break; default: - return false; + return -1; } char src_buf[text_size]; if (!inet_ntop(diag_msg->idiag_family, diag_msg->id.idiag_src, src_buf, text_size)) - return false; + return -1; if (diag_msg->id.idiag_dport || memcmp(zero_addr, diag_msg->id.idiag_dst, addr_size)) { @@ -125,7 +125,7 @@ inet_parse_response(const char *proto_name, const void *data, int data_len, if (!inet_ntop(diag_msg->idiag_family, diag_msg->id.idiag_dst, dst_buf, text_size)) - return false; + return -1; tprintf("%s:[%s:%u->%s:%u]", proto_name, @@ -136,13 +136,14 @@ inet_parse_response(const char *proto_name, const void *data, int data_len, ntohs(diag_msg->id.idiag_sport)); } - return true; + return 1; } static bool receive_responses(const int fd, const unsigned long inode, const char *proto_name, - bool (* parser) (const char *, const void *, int, const unsigned long)) + int (* parser) (const char *, const void *, + int, unsigned long)) { static long buf[8192 / sizeof(long)]; struct sockaddr_nl nladdr = { @@ -156,9 +157,8 @@ receive_responses(const int fd, const unsigned long inode, for (;;) { ssize_t ret; - struct nlmsghdr *h; struct msghdr msg = { - .msg_name = (void*)&nladdr, + .msg_name = (void *) &nladdr, .msg_namelen = sizeof(nladdr), .msg_iov = &iov, .msg_iovlen = 1 @@ -170,18 +170,19 @@ receive_responses(const int fd, const unsigned long inode, continue; return false; } - if (!ret) + + struct nlmsghdr *h = (struct nlmsghdr *) buf; + if (!NLMSG_OK(h, ret)) return false; - for (h = (struct nlmsghdr*)buf; - NLMSG_OK(h, ret); - h = NLMSG_NEXT(h, ret)) { - switch (h->nlmsg_type) { - case NLMSG_DONE: - case NLMSG_ERROR: - return false; - } - if (parser(proto_name, NLMSG_DATA(h), h->nlmsg_len, inode)) + for (; NLMSG_OK(h, ret); h = NLMSG_NEXT(h, ret)) { + if (h->nlmsg_type != SOCK_DIAG_BY_FAMILY) + return false; + int rc = parser(proto_name, NLMSG_DATA(h), + h->nlmsg_len, inode); + if (rc > 0) return true; + if (rc < 0) + return false; } flags = MSG_DONTWAIT; } @@ -222,7 +223,7 @@ unix_send_query(const int fd, const unsigned long inode) .iov_len = sizeof(req) }; struct msghdr msg = { - .msg_name = (void*)&nladdr, + .msg_name = (void *) &nladdr, .msg_namelen = sizeof(nladdr), .msg_iov = &iov, .msg_iovlen = 1 @@ -238,9 +239,9 @@ unix_send_query(const int fd, const unsigned long inode) } } -static bool -unix_parse_response(const char *proto_name, const void *data, int data_len, - const unsigned long inode) +static int +unix_parse_response(const char *proto_name, const void *data, + const int data_len, const unsigned long inode) { const struct unix_diag_msg *diag_msg = data; struct rtattr *attr; @@ -250,11 +251,11 @@ unix_parse_response(const char *proto_name, const void *data, int data_len, char path[UNIX_PATH_MAX + 1]; if (rta_len < 0) - return false; + return -1; if (diag_msg->udiag_ino != inode) - return false; + return 0; if (diag_msg->udiag_family != AF_UNIX) - return false; + return -1; for (attr = (struct rtattr *) (diag_msg + 1); RTA_OK(attr, rta_len); @@ -271,7 +272,7 @@ unix_parse_response(const char *proto_name, const void *data, int data_len, break; case UNIX_DIAG_PEER: if (RTA_PAYLOAD(attr) >= 4) - peer = *(uint32_t *)RTA_DATA(attr); + peer = *(uint32_t *) RTA_DATA(attr); break; } } @@ -296,10 +297,10 @@ unix_parse_response(const char *proto_name, const void *data, int data_len, } } tprints("]"); - return true; + return 1; } else - return false; + return -1; } static bool diff --git a/tests/netlink_inet_diag.c b/tests/netlink_inet_diag.c index 8dd50dba..3754d1bd 100644 --- a/tests/netlink_inet_diag.c +++ b/tests/netlink_inet_diag.c @@ -1,4 +1,6 @@ /* + * This file is part of inet-yy strace test. + * * Copyright (c) 2014-2016 Dmitry V. Levin * All rights reserved. * @@ -27,6 +29,7 @@ #include "tests.h" #include +#include #include #include #include @@ -60,7 +63,7 @@ send_query(const int fd, const int family, const int proto) .iov_len = sizeof(req) }; struct msghdr msg = { - .msg_name = (void*)&nladdr, + .msg_name = (void *) &nladdr, .msg_namelen = sizeof(nladdr), .msg_iov = &iov, .msg_iovlen = 1 @@ -82,7 +85,7 @@ check_responses(const int fd) .iov_len = sizeof(buf) }; struct msghdr msg = { - .msg_name = (void*)&nladdr, + .msg_name = (void *) &nladdr, .msg_namelen = sizeof(nladdr), .msg_iov = &iov, .msg_iovlen = 1 @@ -92,13 +95,23 @@ check_responses(const int fd) if (ret <= 0) perror_msg_and_skip("recvmsg"); - struct nlmsghdr *h = (struct nlmsghdr*)buf; + struct nlmsghdr *h = (struct nlmsghdr *) buf; if (!NLMSG_OK(h, ret)) error_msg_and_skip("!NLMSG_OK"); - if (h->nlmsg_type == NLMSG_ERROR) - error_msg_and_skip("NLMSG_ERROR"); - if (h->nlmsg_type == NLMSG_DONE) - error_msg_and_skip("NLMSG_DONE"); + if (h->nlmsg_type == NLMSG_ERROR) { + const struct nlmsgerr *err = NLMSG_DATA(h); + if (h->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) + error_msg_and_skip("NLMSG_ERROR"); + errno = -err->error; + perror_msg_and_skip("NLMSG_ERROR"); + } + if (h->nlmsg_type != SOCK_DIAG_BY_FAMILY) + error_msg_and_skip("unexpected nlmsg_type %u", + (unsigned) h->nlmsg_type); + + const struct inet_diag_msg *diag = NLMSG_DATA(h); + if (h->nlmsg_len < NLMSG_LENGTH(sizeof(*diag))) + error_msg_and_skip("short response"); } int main(void) diff --git a/tests/netlink_unix_diag.c b/tests/netlink_unix_diag.c index 6e3b63b2..12033e0f 100644 --- a/tests/netlink_unix_diag.c +++ b/tests/netlink_unix_diag.c @@ -1,4 +1,6 @@ /* + * This file is part of net-yy-unix strace test. + * * Copyright (c) 2014-2016 Dmitry V. Levin * All rights reserved. * @@ -27,6 +29,7 @@ #include "tests.h" #include +#include #include #include #include @@ -68,7 +71,7 @@ send_query(const int fd, const int family, const int proto) .iov_len = sizeof(req) }; struct msghdr msg = { - .msg_name = (void*)&nladdr, + .msg_name = (void *) &nladdr, .msg_namelen = sizeof(nladdr), .msg_iov = &iov, .msg_iovlen = 1 @@ -90,7 +93,7 @@ check_responses(const int fd) .iov_len = sizeof(buf) }; struct msghdr msg = { - .msg_name = (void*)&nladdr, + .msg_name = (void *) &nladdr, .msg_namelen = sizeof(nladdr), .msg_iov = &iov, .msg_iovlen = 1 @@ -100,13 +103,23 @@ check_responses(const int fd) if (ret <= 0) perror_msg_and_skip("recvmsg"); - struct nlmsghdr *h = (struct nlmsghdr*)buf; + struct nlmsghdr *h = (struct nlmsghdr *) buf; if (!NLMSG_OK(h, ret)) error_msg_and_skip("!NLMSG_OK"); - if (h->nlmsg_type == NLMSG_ERROR) - error_msg_and_skip("NLMSG_ERROR"); - if (h->nlmsg_type == NLMSG_DONE) - error_msg_and_skip("NLMSG_DONE"); + if (h->nlmsg_type == NLMSG_ERROR) { + const struct nlmsgerr *err = NLMSG_DATA(h); + if (h->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) + error_msg_and_skip("NLMSG_ERROR"); + errno = -err->error; + perror_msg_and_skip("NLMSG_ERROR"); + } + if (h->nlmsg_type != SOCK_DIAG_BY_FAMILY) + error_msg_and_skip("unexpected nlmsg_type %u", + (unsigned) h->nlmsg_type); + + const struct unix_diag_msg *diag = NLMSG_DATA(h); + if (h->nlmsg_len < NLMSG_LENGTH(sizeof(*diag))) + error_msg_and_skip("short response"); } #define SUN_PATH "netlink_unix_diag_socket" -- 2.40.0