* [PATCH v1 0/2] Fix -Werror=array-bounds errors
@ 2021-06-22 7:49 Daniel Wagner
2021-06-22 7:49 ` [PATCH v1 1/2] rtnl: Fix netlink message alignment Daniel Wagner
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Daniel Wagner @ 2021-06-22 7:49 UTC (permalink / raw)
To: connman; +Cc: Daniel Wagner
gcc complains with
src/rtnl.c: In function ‘send_getlink’:
src/rtnl.c:1485:12: error: array subscript ‘struct rtnl_request[0]’ is partly outside array bounds of ‘unsigned char[17]’ [-Werror=array-bounds]
1485 | req->hdr.nlmsg_len = RTNL_REQUEST_SIZE;
| ^~
The netlink helper 'struct rtnl_request' doesn't include any padding
between header and message. This is wrong. Fix this by removing the
helper structure and open code the message generation.
Daniel Wagner (2):
rtnl: Fix netlink message alignment
vpn-rtnl: Fix netlink message alignment
src/rtnl.c | 125 ++++++++++++++++++++++++-------------------------
vpn/vpn-rtnl.c | 125 ++++++++++++++++++++++++-------------------------
2 files changed, 124 insertions(+), 126 deletions(-)
--
2.32.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v1 1/2] rtnl: Fix netlink message alignment
2021-06-22 7:49 [PATCH v1 0/2] Fix -Werror=array-bounds errors Daniel Wagner
@ 2021-06-22 7:49 ` Daniel Wagner
2021-06-22 7:49 ` [PATCH v1 2/2] vpn-rtnl: " Daniel Wagner
2021-06-23 7:38 ` [PATCH v1 0/2] Fix -Werror=array-bounds errors Daniel Wagner
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Wagner @ 2021-06-22 7:49 UTC (permalink / raw)
To: connman; +Cc: Daniel Wagner
Recent versions of gcc complain with outside of bounds access.
Between the netlink header and the message there is padding. Refactor
the code and open code the message generation.
---
src/rtnl.c | 125 ++++++++++++++++++++++++++---------------------------
1 file changed, 62 insertions(+), 63 deletions(-)
diff --git a/src/rtnl.c b/src/rtnl.c
index c9e84daba458..a87296ffe921 100644
--- a/src/rtnl.c
+++ b/src/rtnl.c
@@ -1308,75 +1308,71 @@ static const char *type2string(uint16_t type)
static GIOChannel *channel = NULL;
static guint channel_watch = 0;
-struct rtnl_request {
- struct nlmsghdr hdr;
- struct rtgenmsg msg;
-};
-#define RTNL_REQUEST_SIZE (sizeof(struct nlmsghdr) + sizeof(struct rtgenmsg))
+#define RTNL_REQUEST_SIZE (NLMSG_HDRLEN + NLMSG_ALIGN(sizeof(struct rtgenmsg)))
static GSList *request_list = NULL;
static guint32 request_seq = 0;
-static struct rtnl_request *find_request(guint32 seq)
+static struct nlmsghdr *find_request(guint32 seq)
{
GSList *list;
for (list = request_list; list; list = list->next) {
- struct rtnl_request *req = list->data;
+ struct nlmsghdr *hdr = list->data;
- if (req->hdr.nlmsg_seq == seq)
- return req;
+ if (hdr->nlmsg_seq == seq)
+ return hdr;
}
return NULL;
}
-static int send_request(struct rtnl_request *req)
+static int send_request(struct nlmsghdr *hdr)
{
struct sockaddr_nl addr;
int sk;
DBG("%s len %d type %d flags 0x%04x seq %d",
- type2string(req->hdr.nlmsg_type),
- req->hdr.nlmsg_len, req->hdr.nlmsg_type,
- req->hdr.nlmsg_flags, req->hdr.nlmsg_seq);
+ type2string(hdr->nlmsg_type),
+ hdr->nlmsg_len, hdr->nlmsg_type,
+ hdr->nlmsg_flags, hdr->nlmsg_seq);
sk = g_io_channel_unix_get_fd(channel);
memset(&addr, 0, sizeof(addr));
addr.nl_family = AF_NETLINK;
- return sendto(sk, req, req->hdr.nlmsg_len, 0,
+ return sendto(sk, hdr, hdr->nlmsg_len, 0,
(struct sockaddr *) &addr, sizeof(addr));
}
-static int queue_request(struct rtnl_request *req)
+static int queue_request(struct nlmsghdr *hdr)
{
- request_list = g_slist_append(request_list, req);
+ request_list = g_slist_append(request_list, hdr);
if (g_slist_length(request_list) > 1)
return 0;
- return send_request(req);
+ return send_request(hdr);
}
static int process_response(guint32 seq)
{
- struct rtnl_request *req;
+ struct nlmsghdr *hdr;
DBG("seq %d", seq);
- req = find_request(seq);
- if (req) {
- request_list = g_slist_remove(request_list, req);
- g_free(req);
+ hdr = find_request(seq);
+ if (hdr) {
+ request_list = g_slist_remove(request_list, hdr);
+ g_free(hdr);
}
- req = g_slist_nth_data(request_list, 0);
- if (!req)
+ hdr = g_slist_nth_data(request_list, 0);
+ if (!hdr)
return 0;
- return send_request(req);
+ return send_request(hdr);
}
static void rtnl_message(void *buf, size_t len)
@@ -1474,62 +1470,65 @@ static gboolean netlink_event(GIOChannel *chan, GIOCondition cond, gpointer data
static int send_getlink(void)
{
- struct rtnl_request *req;
+ struct nlmsghdr *hdr;
+ struct rtgenmsg *msg;
DBG("");
- req = g_try_malloc0(RTNL_REQUEST_SIZE);
- if (!req)
- return -ENOMEM;
+ hdr = g_malloc0(RTNL_REQUEST_SIZE);
+
+ hdr->nlmsg_len = RTNL_REQUEST_SIZE;
+ hdr->nlmsg_type = RTM_GETLINK;
+ hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+ hdr->nlmsg_pid = 0;
+ hdr->nlmsg_seq = request_seq++;
- req->hdr.nlmsg_len = RTNL_REQUEST_SIZE;
- req->hdr.nlmsg_type = RTM_GETLINK;
- req->hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
- req->hdr.nlmsg_pid = 0;
- req->hdr.nlmsg_seq = request_seq++;
- req->msg.rtgen_family = AF_INET;
+ msg = (struct rtgenmsg *) NLMSG_DATA(hdr);
+ msg->rtgen_family = AF_INET;
- return queue_request(req);
+ return queue_request(hdr);
}
static int send_getaddr(void)
{
- struct rtnl_request *req;
+ struct nlmsghdr *hdr;
+ struct rtgenmsg *msg;
DBG("");
- req = g_try_malloc0(RTNL_REQUEST_SIZE);
- if (!req)
- return -ENOMEM;
+ hdr = g_malloc0(RTNL_REQUEST_SIZE);
- req->hdr.nlmsg_len = RTNL_REQUEST_SIZE;
- req->hdr.nlmsg_type = RTM_GETADDR;
- req->hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
- req->hdr.nlmsg_pid = 0;
- req->hdr.nlmsg_seq = request_seq++;
- req->msg.rtgen_family = AF_INET;
+ hdr->nlmsg_len = RTNL_REQUEST_SIZE;
+ hdr->nlmsg_type = RTM_GETADDR;
+ hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+ hdr->nlmsg_pid = 0;
+ hdr->nlmsg_seq = request_seq++;
- return queue_request(req);
+ msg = (struct rtgenmsg *) NLMSG_DATA(hdr);
+ msg->rtgen_family = AF_INET;
+
+ return queue_request(hdr);
}
static int send_getroute(void)
{
- struct rtnl_request *req;
+ struct nlmsghdr *hdr;
+ struct rtgenmsg *msg;
DBG("");
- req = g_try_malloc0(RTNL_REQUEST_SIZE);
- if (!req)
- return -ENOMEM;
+ hdr = g_malloc0(RTNL_REQUEST_SIZE);
+
+ hdr->nlmsg_len = RTNL_REQUEST_SIZE;
+ hdr->nlmsg_type = RTM_GETROUTE;
+ hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+ hdr->nlmsg_pid = 0;
+ hdr->nlmsg_seq = request_seq++;
- req->hdr.nlmsg_len = RTNL_REQUEST_SIZE;
- req->hdr.nlmsg_type = RTM_GETROUTE;
- req->hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
- req->hdr.nlmsg_pid = 0;
- req->hdr.nlmsg_seq = request_seq++;
- req->msg.rtgen_family = AF_INET;
+ msg = (struct rtgenmsg *) NLMSG_DATA(hdr);
+ msg->rtgen_family = AF_INET;
- return queue_request(req);
+ return queue_request(hdr);
}
static gboolean update_timeout_cb(gpointer user_data)
@@ -1673,14 +1672,14 @@ void __connman_rtnl_cleanup(void)
update_list = NULL;
for (list = request_list; list; list = list->next) {
- struct rtnl_request *req = list->data;
+ struct nlmsghdr *hdr= list->data;
DBG("%s len %d type %d flags 0x%04x seq %d",
- type2string(req->hdr.nlmsg_type),
- req->hdr.nlmsg_len, req->hdr.nlmsg_type,
- req->hdr.nlmsg_flags, req->hdr.nlmsg_seq);
+ type2string(hdr->nlmsg_type),
+ hdr->nlmsg_len, hdr->nlmsg_type,
+ hdr->nlmsg_flags, hdr->nlmsg_seq);
- g_free(req);
+ g_free(hdr);
list->data = NULL;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v1 2/2] vpn-rtnl: Fix netlink message alignment
2021-06-22 7:49 [PATCH v1 0/2] Fix -Werror=array-bounds errors Daniel Wagner
2021-06-22 7:49 ` [PATCH v1 1/2] rtnl: Fix netlink message alignment Daniel Wagner
@ 2021-06-22 7:49 ` Daniel Wagner
2021-06-23 7:38 ` [PATCH v1 0/2] Fix -Werror=array-bounds errors Daniel Wagner
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Wagner @ 2021-06-22 7:49 UTC (permalink / raw)
To: connman; +Cc: Daniel Wagner
Recent versions of gcc complain with outside of bounds access.
Between the netlink header and the message there is padding. Refactor
the code and open code the message generation.
---
vpn/vpn-rtnl.c | 125 ++++++++++++++++++++++++-------------------------
1 file changed, 62 insertions(+), 63 deletions(-)
diff --git a/vpn/vpn-rtnl.c b/vpn/vpn-rtnl.c
index 295c05ce5363..5a02d779a0ab 100644
--- a/vpn/vpn-rtnl.c
+++ b/vpn/vpn-rtnl.c
@@ -797,75 +797,71 @@ static const char *type2string(uint16_t type)
static GIOChannel *channel = NULL;
-struct rtnl_request {
- struct nlmsghdr hdr;
- struct rtgenmsg msg;
-};
-#define RTNL_REQUEST_SIZE (sizeof(struct nlmsghdr) + sizeof(struct rtgenmsg))
+#define RTNL_REQUEST_SIZE (NLMSG_HDRLEN + NLMSG_ALIGN(sizeof(struct rtgenmsg)))
static GSList *request_list = NULL;
static guint32 request_seq = 0;
-static struct rtnl_request *find_request(guint32 seq)
+static struct nlmsghdr *find_request(guint32 seq)
{
GSList *list;
for (list = request_list; list; list = list->next) {
- struct rtnl_request *req = list->data;
+ struct nlmsghdr *hdr = list->data;
- if (req->hdr.nlmsg_seq == seq)
- return req;
+ if (hdr->nlmsg_seq == seq)
+ return hdr;
}
return NULL;
}
-static int send_request(struct rtnl_request *req)
+static int send_request(struct nlmsghdr *hdr)
{
struct sockaddr_nl addr;
int sk;
debug("%s len %d type %d flags 0x%04x seq %d",
- type2string(req->hdr.nlmsg_type),
- req->hdr.nlmsg_len, req->hdr.nlmsg_type,
- req->hdr.nlmsg_flags, req->hdr.nlmsg_seq);
+ type2string(hdr->nlmsg_type),
+ hdr->nlmsg_len, hdr->nlmsg_type,
+ hdr->nlmsg_flags, hdr->nlmsg_seq);
sk = g_io_channel_unix_get_fd(channel);
memset(&addr, 0, sizeof(addr));
addr.nl_family = AF_NETLINK;
- return sendto(sk, req, req->hdr.nlmsg_len, 0,
+ return sendto(sk, hdr, hdr->nlmsg_len, 0,
(struct sockaddr *) &addr, sizeof(addr));
}
-static int queue_request(struct rtnl_request *req)
+static int queue_request(struct nlmsghdr *hdr)
{
- request_list = g_slist_append(request_list, req);
+ request_list = g_slist_append(request_list, hdr);
if (g_slist_length(request_list) > 1)
return 0;
- return send_request(req);
+ return send_request(hdr);
}
static int process_response(guint32 seq)
{
- struct rtnl_request *req;
+ struct nlmsghdr *hdr;
debug("seq %d", seq);
- req = find_request(seq);
- if (req) {
- request_list = g_slist_remove(request_list, req);
- g_free(req);
+ hdr = find_request(seq);
+ if (hdr) {
+ request_list = g_slist_remove(request_list, hdr);
+ g_free(hdr);
}
- req = g_slist_nth_data(request_list, 0);
- if (!req)
+ hdr = g_slist_nth_data(request_list, 0);
+ if (!hdr)
return 0;
- return send_request(req);
+ return send_request(hdr);
}
static void rtnl_message(void *buf, size_t len)
@@ -960,62 +956,65 @@ static gboolean netlink_event(GIOChannel *chan, GIOCondition cond, gpointer data
static int send_getlink(void)
{
- struct rtnl_request *req;
+ struct nlmsghdr *hdr;
+ struct rtgenmsg *msg;
debug("");
- req = g_try_malloc0(RTNL_REQUEST_SIZE);
- if (!req)
- return -ENOMEM;
+ hdr = g_malloc0(RTNL_REQUEST_SIZE);
+
+ hdr->nlmsg_len = RTNL_REQUEST_SIZE;
+ hdr->nlmsg_type = RTM_GETLINK;
+ hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+ hdr->nlmsg_pid = 0;
+ hdr->nlmsg_seq = request_seq++;
- req->hdr.nlmsg_len = RTNL_REQUEST_SIZE;
- req->hdr.nlmsg_type = RTM_GETLINK;
- req->hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
- req->hdr.nlmsg_pid = 0;
- req->hdr.nlmsg_seq = request_seq++;
- req->msg.rtgen_family = AF_INET;
+ msg = (struct rtgenmsg *) NLMSG_DATA(hdr);
+ msg->rtgen_family = AF_INET;
- return queue_request(req);
+ return queue_request(hdr);
}
static int send_getaddr(void)
{
- struct rtnl_request *req;
+ struct nlmsghdr *hdr;
+ struct rtgenmsg *msg;
debug("");
- req = g_try_malloc0(RTNL_REQUEST_SIZE);
- if (!req)
- return -ENOMEM;
+ hdr = g_malloc0(RTNL_REQUEST_SIZE);
- req->hdr.nlmsg_len = RTNL_REQUEST_SIZE;
- req->hdr.nlmsg_type = RTM_GETADDR;
- req->hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
- req->hdr.nlmsg_pid = 0;
- req->hdr.nlmsg_seq = request_seq++;
- req->msg.rtgen_family = AF_INET;
+ hdr->nlmsg_len = RTNL_REQUEST_SIZE;
+ hdr->nlmsg_type = RTM_GETADDR;
+ hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+ hdr->nlmsg_pid = 0;
+ hdr->nlmsg_seq = request_seq++;
- return queue_request(req);
+ msg = (struct rtgenmsg *) NLMSG_DATA(hdr);
+ msg->rtgen_family = AF_INET;
+
+ return queue_request(hdr);
}
static int send_getroute(void)
{
- struct rtnl_request *req;
+ struct nlmsghdr *hdr;
+ struct rtgenmsg *msg;
debug("");
- req = g_try_malloc0(RTNL_REQUEST_SIZE);
- if (!req)
- return -ENOMEM;
+ hdr = g_malloc0(RTNL_REQUEST_SIZE);
+
+ hdr->nlmsg_len = RTNL_REQUEST_SIZE;
+ hdr->nlmsg_type = RTM_GETROUTE;
+ hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+ hdr->nlmsg_pid = 0;
+ hdr->nlmsg_seq = request_seq++;
- req->hdr.nlmsg_len = RTNL_REQUEST_SIZE;
- req->hdr.nlmsg_type = RTM_GETROUTE;
- req->hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
- req->hdr.nlmsg_pid = 0;
- req->hdr.nlmsg_seq = request_seq++;
- req->msg.rtgen_family = AF_INET;
+ msg = (struct rtgenmsg *) NLMSG_DATA(hdr);
+ msg->rtgen_family = AF_INET;
- return queue_request(req);
+ return queue_request(hdr);
}
static gboolean update_timeout_cb(gpointer user_data)
@@ -1158,14 +1157,14 @@ void __vpn_rtnl_cleanup(void)
update_list = NULL;
for (list = request_list; list; list = list->next) {
- struct rtnl_request *req = list->data;
+ struct nlmsghdr *hdr = list->data;
debug("%s len %d type %d flags 0x%04x seq %d",
- type2string(req->hdr.nlmsg_type),
- req->hdr.nlmsg_len, req->hdr.nlmsg_type,
- req->hdr.nlmsg_flags, req->hdr.nlmsg_seq);
+ type2string(hdr->nlmsg_type),
+ hdr->nlmsg_len, hdr->nlmsg_type,
+ hdr->nlmsg_flags, hdr->nlmsg_seq);
- g_free(req);
+ g_free(hdr);
list->data = NULL;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1 0/2] Fix -Werror=array-bounds errors
2021-06-22 7:49 [PATCH v1 0/2] Fix -Werror=array-bounds errors Daniel Wagner
2021-06-22 7:49 ` [PATCH v1 1/2] rtnl: Fix netlink message alignment Daniel Wagner
2021-06-22 7:49 ` [PATCH v1 2/2] vpn-rtnl: " Daniel Wagner
@ 2021-06-23 7:38 ` Daniel Wagner
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Wagner @ 2021-06-23 7:38 UTC (permalink / raw)
To: connman
On Tue, Jun 22, 2021 at 09:49:31AM +0200, Daniel Wagner wrote:
> gcc complains with
>
> src/rtnl.c: In function ‘send_getlink’:
> src/rtnl.c:1485:12: error: array subscript ‘struct rtnl_request[0]’ is partly outside array bounds of ‘unsigned char[17]’ [-Werror=array-bounds]
> 1485 | req->hdr.nlmsg_len = RTNL_REQUEST_SIZE;
> | ^~
>
>
> The netlink helper 'struct rtnl_request' doesn't include any padding
> between header and message. This is wrong. Fix this by removing the
> helper structure and open code the message generation.
Patches applied
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-23 7:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-22 7:49 [PATCH v1 0/2] Fix -Werror=array-bounds errors Daniel Wagner
2021-06-22 7:49 ` [PATCH v1 1/2] rtnl: Fix netlink message alignment Daniel Wagner
2021-06-22 7:49 ` [PATCH v1 2/2] vpn-rtnl: " Daniel Wagner
2021-06-23 7:38 ` [PATCH v1 0/2] Fix -Werror=array-bounds errors Daniel Wagner
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).