[v2] common/mlx5: fix netlink buffer allocation from stack
Checks
Commit Message
The buffer size to receive netlink reply messages is relatively
large (32K), and it is allocated on the stack and it might
break in application is using smaller per-thread stacks.
This patch allocates temporary buffer from heap.
Fixes: ccdcba53a3f4 ("net/mlx5: use Netlink to add/remove MAC addresses")
Cc: nelio.laranjeiro@6wind.com
Cc: stable@dpdk.org
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
v2: allocation from heap, instead of reducing buffer size
v1: http://patches.dpdk.org/patch/69158/
---
drivers/common/mlx5/mlx5_nl.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
Comments
From: Viacheslav Ovsiienko
> The buffer size to receive netlink reply messages is relatively large (32K), and
> it is allocated on the stack and it might break in application is using smaller
> per-thread stacks.
> This patch allocates temporary buffer from heap.
>
> Fixes: ccdcba53a3f4 ("net/mlx5: use Netlink to add/remove MAC addresses")
> Cc: nelio.laranjeiro@6wind.com
> Cc: stable@dpdk.org
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
Hi,
> -----Original Message-----
> From: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Sent: Thursday, May 14, 2020 10:11 AM
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; stephen@networkplumber.org; Nélio Laranjeiro
> <nelio.laranjeiro@6wind.com>; stable@dpdk.org
> Subject: [PATCH v2] common/mlx5: fix netlink buffer allocation from stack
>
> The buffer size to receive netlink reply messages is relatively
> large (32K), and it is allocated on the stack and it might
> break in application is using smaller per-thread stacks.
> This patch allocates temporary buffer from heap.
>
> Fixes: ccdcba53a3f4 ("net/mlx5: use Netlink to add/remove MAC addresses")
> Cc: nelio.laranjeiro@6wind.com
> Cc: stable@dpdk.org
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>
> ---
> v2: allocation from heap, instead of reducing buffer size
> v1:
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> es.dpdk.org%2Fpatch%2F69158%2F&data=02%7C01%7Crasland%40mell
> anox.com%7C1e38e272cede42d2cfc008d7f7d60e00%7Ca652971c7d2e4d9ba6
> a4d149256f461b%7C0%7C0%7C637250371034928200&sdata=cZ1hFsnSZz
> 5oMj8zOLbB6vmcthY1FnPZWa%2FgoNDibmg%3D&reserved=0
> ---
> drivers/common/mlx5/mlx5_nl.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/common/mlx5/mlx5_nl.c
> b/drivers/common/mlx5/mlx5_nl.c
> index 65efcd3..1a1033a 100644
> --- a/drivers/common/mlx5/mlx5_nl.c
> +++ b/drivers/common/mlx5/mlx5_nl.c
> @@ -330,10 +330,10 @@ struct mlx5_nl_ifindex_data {
> void *arg)
> {
> struct sockaddr_nl sa;
> - char buf[MLX5_RECV_BUF_SIZE];
> + void *buf = malloc(MLX5_RECV_BUF_SIZE);
> struct iovec iov = {
> .iov_base = buf,
> - .iov_len = sizeof(buf),
> + .iov_len = MLX5_RECV_BUF_SIZE,
> };
> struct msghdr msg = {
> .msg_name = &sa,
> @@ -345,6 +345,10 @@ struct mlx5_nl_ifindex_data {
> int multipart = 0;
> int ret = 0;
>
> + if (!buf) {
> + rte_errno = ENOMEM;
> + return -rte_errno;
> + }
> do {
> struct nlmsghdr *nh;
> int recv_bytes = 0;
> @@ -353,7 +357,8 @@ struct mlx5_nl_ifindex_data {
> recv_bytes = recvmsg(nlsk_fd, &msg, 0);
> if (recv_bytes == -1) {
> rte_errno = errno;
> - return -rte_errno;
> + ret = -rte_errno;
> + goto exit;
> }
> nh = (struct nlmsghdr *)buf;
> } while (nh->nlmsg_seq != sn);
> @@ -365,24 +370,30 @@ struct mlx5_nl_ifindex_data {
>
> if (err_data->error < 0) {
> rte_errno = -err_data->error;
> - return -rte_errno;
> + ret = -rte_errno;
> + goto exit;
> }
> /* Ack message. */
> - return 0;
> + ret = 0;
> + goto exit;
> }
> /* Multi-part msgs and their trailing DONE message.
> */
> if (nh->nlmsg_flags & NLM_F_MULTI) {
> - if (nh->nlmsg_type == NLMSG_DONE)
> - return 0;
> + if (nh->nlmsg_type == NLMSG_DONE) {
> + ret = 0;
> + goto exit;
> + }
> multipart = 1;
> }
> if (cb) {
> ret = cb(nh, arg);
> if (ret < 0)
> - return ret;
> + goto exit;
> }
> }
> } while (multipart);
> +exit:
> + free(buf);
> return ret;
> }
>
> --
> 1.8.3.1
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
@@ -330,10 +330,10 @@ struct mlx5_nl_ifindex_data {
void *arg)
{
struct sockaddr_nl sa;
- char buf[MLX5_RECV_BUF_SIZE];
+ void *buf = malloc(MLX5_RECV_BUF_SIZE);
struct iovec iov = {
.iov_base = buf,
- .iov_len = sizeof(buf),
+ .iov_len = MLX5_RECV_BUF_SIZE,
};
struct msghdr msg = {
.msg_name = &sa,
@@ -345,6 +345,10 @@ struct mlx5_nl_ifindex_data {
int multipart = 0;
int ret = 0;
+ if (!buf) {
+ rte_errno = ENOMEM;
+ return -rte_errno;
+ }
do {
struct nlmsghdr *nh;
int recv_bytes = 0;
@@ -353,7 +357,8 @@ struct mlx5_nl_ifindex_data {
recv_bytes = recvmsg(nlsk_fd, &msg, 0);
if (recv_bytes == -1) {
rte_errno = errno;
- return -rte_errno;
+ ret = -rte_errno;
+ goto exit;
}
nh = (struct nlmsghdr *)buf;
} while (nh->nlmsg_seq != sn);
@@ -365,24 +370,30 @@ struct mlx5_nl_ifindex_data {
if (err_data->error < 0) {
rte_errno = -err_data->error;
- return -rte_errno;
+ ret = -rte_errno;
+ goto exit;
}
/* Ack message. */
- return 0;
+ ret = 0;
+ goto exit;
}
/* Multi-part msgs and their trailing DONE message. */
if (nh->nlmsg_flags & NLM_F_MULTI) {
- if (nh->nlmsg_type == NLMSG_DONE)
- return 0;
+ if (nh->nlmsg_type == NLMSG_DONE) {
+ ret = 0;
+ goto exit;
+ }
multipart = 1;
}
if (cb) {
ret = cb(nh, arg);
if (ret < 0)
- return ret;
+ goto exit;
}
}
} while (multipart);
+exit:
+ free(buf);
return ret;
}