* [PATCH iproute2 2/3] rdma: stat: fix memory leak in res_counter_line()
2022-02-01 17:39 [PATCH iproute2 0/3] some memory leak fixes Andrea Claudi
2022-02-01 17:39 ` [PATCH iproute2 1/3] lib/fs: fix memory leak in get_task_name() Andrea Claudi
@ 2022-02-01 17:39 ` Andrea Claudi
2022-02-01 17:39 ` [PATCH iproute2 3/3] rdma: RES_PID and RES_KERN_NAME are alternatives to each other Andrea Claudi
2 siblings, 0 replies; 7+ messages in thread
From: Andrea Claudi @ 2022-02-01 17:39 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, markzhang, leonro
get_task_name() uses asprintf() to reserve storage for the task name
string. As asprintf() manpage states, a free() should be used to release
the allocated storage when it is no longer needed.
res_counter_line() uses get_task_name() but does not free the allocated
storage on the error paths and after the usage.
This uses a single return point to free the allocated storage, adopting
the same approach of other rdma code using get_task_name().
Fixes: 5937552b42e4 ("rdma: Add "stat qp show" support")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
rdma/stat.c | 74 ++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 50 insertions(+), 24 deletions(-)
diff --git a/rdma/stat.c b/rdma/stat.c
index adfcd34a..66121b12 100644
--- a/rdma/stat.c
+++ b/rdma/stat.c
@@ -222,9 +222,9 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
uint32_t cntn, port = 0, pid = 0, qpn, qp_type = 0;
struct nlattr *hwc_table, *qp_table;
struct nlattr *nla_entry;
- const char *comm = NULL;
+ char *comm = NULL;
bool isfirst;
- int err;
+ int err, ret;
if (nla_line[RDMA_NLDEV_ATTR_PORT_INDEX])
port = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_PORT_INDEX]);
@@ -232,52 +232,70 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
hwc_table = nla_line[RDMA_NLDEV_ATTR_STAT_HWCOUNTERS];
qp_table = nla_line[RDMA_NLDEV_ATTR_RES_QP];
if (!hwc_table || !qp_table ||
- !nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID])
- return MNL_CB_ERROR;
+ !nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID]) {
+ ret = MNL_CB_ERROR;
+ goto out;
+ }
cntn = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID]);
if (rd_is_filtered_attr(rd, "cntn", cntn,
- nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID]))
- return MNL_CB_OK;
+ nla_line[RDMA_NLDEV_ATTR_STAT_COUNTER_ID])) {
+ ret = MNL_CB_OK;
+ goto out;
+ }
if (nla_line[RDMA_NLDEV_ATTR_RES_TYPE])
qp_type = mnl_attr_get_u8(nla_line[RDMA_NLDEV_ATTR_RES_TYPE]);
if (rd_is_string_filtered_attr(rd, "qp-type", qp_types_to_str(qp_type),
- nla_line[RDMA_NLDEV_ATTR_RES_TYPE]))
- return MNL_CB_OK;
+ nla_line[RDMA_NLDEV_ATTR_RES_TYPE])) {
+ ret = MNL_CB_OK;
+ goto out;
+ }
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
}
if (rd_is_filtered_attr(rd, "pid", pid,
- nla_line[RDMA_NLDEV_ATTR_RES_PID]))
- return MNL_CB_OK;
+ nla_line[RDMA_NLDEV_ATTR_RES_PID])) {
+ ret = MNL_CB_OK;
+ goto out;
+ }
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
+ if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
comm = (char *)mnl_attr_get_str(
nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
+ }
mnl_attr_for_each_nested(nla_entry, qp_table) {
struct nlattr *qp_line[RDMA_NLDEV_ATTR_MAX] = {};
err = mnl_attr_parse_nested(nla_entry, rd_attr_cb, qp_line);
- if (err != MNL_CB_OK)
- return -EINVAL;
+ if (err != MNL_CB_OK) {
+ ret = -EINVAL;
+ goto out;
+ }
- if (!qp_line[RDMA_NLDEV_ATTR_RES_LQPN])
- return -EINVAL;
+ if (!qp_line[RDMA_NLDEV_ATTR_RES_LQPN]) {
+ ret = -EINVAL;
+ goto out;
+ }
qpn = mnl_attr_get_u32(qp_line[RDMA_NLDEV_ATTR_RES_LQPN]);
if (rd_is_filtered_attr(rd, "lqpn", qpn,
- qp_line[RDMA_NLDEV_ATTR_RES_LQPN]))
- return MNL_CB_OK;
+ qp_line[RDMA_NLDEV_ATTR_RES_LQPN])) {
+ ret = MNL_CB_OK;
+ goto out;
+ }
}
err = res_get_hwcounters(rd, hwc_table, false);
- if (err != MNL_CB_OK)
- return err;
+ if (err != MNL_CB_OK) {
+ ret = err;
+ goto out;
+ }
open_json_object(NULL);
print_link(rd, index, name, port, nla_line);
print_color_uint(PRINT_ANY, COLOR_NONE, "cntn", "cntn %u ", cntn);
@@ -292,11 +310,15 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
mnl_attr_for_each_nested(nla_entry, qp_table) {
struct nlattr *qp_line[RDMA_NLDEV_ATTR_MAX] = {};
err = mnl_attr_parse_nested(nla_entry, rd_attr_cb, qp_line);
- if (err != MNL_CB_OK)
- return -EINVAL;
+ if (err != MNL_CB_OK) {
+ ret = -EINVAL;
+ goto out;
+ }
- if (!qp_line[RDMA_NLDEV_ATTR_RES_LQPN])
- return -EINVAL;
+ if (!qp_line[RDMA_NLDEV_ATTR_RES_LQPN]) {
+ ret = -EINVAL;
+ goto out;
+ }
qpn = mnl_attr_get_u32(qp_line[RDMA_NLDEV_ATTR_RES_LQPN]);
if (!isfirst)
@@ -307,7 +329,11 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
}
close_json_array(PRINT_ANY, ">");
newline(rd);
- return MNL_CB_OK;
+ ret = MNL_CB_OK;
+out:
+ if (nla_line[RDMA_NLDEV_ATTR_RES_PID])
+ free(comm);
+ return ret;
}
static int stat_qp_show_parse_cb(const struct nlmsghdr *nlh, void *data)
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH iproute2 3/3] rdma: RES_PID and RES_KERN_NAME are alternatives to each other
2022-02-01 17:39 [PATCH iproute2 0/3] some memory leak fixes Andrea Claudi
2022-02-01 17:39 ` [PATCH iproute2 1/3] lib/fs: fix memory leak in get_task_name() Andrea Claudi
2022-02-01 17:39 ` [PATCH iproute2 2/3] rdma: stat: fix memory leak in res_counter_line() Andrea Claudi
@ 2022-02-01 17:39 ` Andrea Claudi
2 siblings, 0 replies; 7+ messages in thread
From: Andrea Claudi @ 2022-02-01 17:39 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, markzhang, leonro
RDMA_NLDEV_ATTR_RES_PID and RDMA_NLDEV_ATTR_RES_KERN_NAME cannot be set
together, as evident from the fill_res_name_pid() function in the kernel
infiniband driver. This commit makes this clear at first glance, using
an else branch for RDMA_NLDEV_ATTR_RES_KERN_NAME.
This also helps coverity to better understand this code and avoid
producing a bogus warning complaining about mnl_attr_get_str overwriting
comm, and thus leaking the storage that comm points to.
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
rdma/res-cmid.c | 10 ++++------
rdma/res-cq.c | 9 ++++-----
rdma/res-ctx.c | 9 ++++-----
rdma/res-mr.c | 8 ++++----
rdma/res-pd.c | 9 ++++-----
rdma/res-qp.c | 9 ++++-----
rdma/res-srq.c | 10 +++++-----
rdma/stat.c | 11 +++++------
8 files changed, 34 insertions(+), 41 deletions(-)
diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c
index bfaa47b5..21bdabca 100644
--- a/rdma/res-cmid.c
+++ b/rdma/res-cmid.c
@@ -161,6 +161,10 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx,
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
+ } else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
+ comm = (char *)mnl_attr_get_str(
+ nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
}
if (rd_is_filtered_attr(rd, "pid", pid,
@@ -173,12 +177,6 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx,
nla_line[RDMA_NLDEV_ATTR_RES_CM_IDN]))
goto out;
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
- /* discard const from mnl_attr_get_str */
- comm = (char *)mnl_attr_get_str(
- nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
- }
-
open_json_object(NULL);
print_link(rd, idx, name, port, nla_line);
res_print_uint(rd, "cm-idn", cm_idn,
diff --git a/rdma/res-cq.c b/rdma/res-cq.c
index 9e7c4f51..0ba19447 100644
--- a/rdma/res-cq.c
+++ b/rdma/res-cq.c
@@ -86,6 +86,10 @@ static int res_cq_line(struct rd *rd, const char *name, int idx,
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
+ } else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
+ comm = (char *)mnl_attr_get_str(
+ nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
}
if (rd_is_filtered_attr(rd, "pid", pid,
@@ -103,11 +107,6 @@ static int res_cq_line(struct rd *rd, const char *name, int idx,
nla_line[RDMA_NLDEV_ATTR_RES_CTXN]))
goto out;
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
- /* discard const from mnl_attr_get_str */
- comm = (char *)mnl_attr_get_str(
- nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
open_json_object(NULL);
print_dev(rd, idx, name);
res_print_uint(rd, "cqn", cqn, nla_line[RDMA_NLDEV_ATTR_RES_CQN]);
diff --git a/rdma/res-ctx.c b/rdma/res-ctx.c
index 30afe97a..895598d8 100644
--- a/rdma/res-ctx.c
+++ b/rdma/res-ctx.c
@@ -20,6 +20,10 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx,
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
+ } else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
+ comm = (char *)mnl_attr_get_str(
+ nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
}
if (rd_is_filtered_attr(rd, "pid", pid,
@@ -33,11 +37,6 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx,
nla_line[RDMA_NLDEV_ATTR_RES_CTXN]))
goto out;
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
- /* discard const from mnl_attr_get_str */
- comm = (char *)mnl_attr_get_str(
- nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
open_json_object(NULL);
print_dev(rd, idx, name);
res_print_uint(rd, "ctxn", ctxn, nla_line[RDMA_NLDEV_ATTR_RES_CTXN]);
diff --git a/rdma/res-mr.c b/rdma/res-mr.c
index 1bf73f3a..9ae6c777 100644
--- a/rdma/res-mr.c
+++ b/rdma/res-mr.c
@@ -49,6 +49,10 @@ static int res_mr_line(struct rd *rd, const char *name, int idx,
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
+ } else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
+ comm = (char *)mnl_attr_get_str(
+ nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
}
if (rd_is_filtered_attr(rd, "pid", pid,
@@ -67,10 +71,6 @@ static int res_mr_line(struct rd *rd, const char *name, int idx,
nla_line[RDMA_NLDEV_ATTR_RES_PDN]))
goto out;
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
- /* discard const from mnl_attr_get_str */
- comm = (char *)mnl_attr_get_str(
- nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
open_json_object(NULL);
print_dev(rd, idx, name);
res_print_uint(rd, "mrn", mrn, nla_line[RDMA_NLDEV_ATTR_RES_MRN]);
diff --git a/rdma/res-pd.c b/rdma/res-pd.c
index df538010..aca2b0cc 100644
--- a/rdma/res-pd.c
+++ b/rdma/res-pd.c
@@ -36,6 +36,10 @@ static int res_pd_line(struct rd *rd, const char *name, int idx,
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
+ } else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
+ comm = (char *)mnl_attr_get_str(
+ nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
}
if (rd_is_filtered_attr(rd, "pid", pid,
@@ -55,11 +59,6 @@ static int res_pd_line(struct rd *rd, const char *name, int idx,
nla_line[RDMA_NLDEV_ATTR_RES_PDN]))
goto out;
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
- /* discard const from mnl_attr_get_str */
- comm = (char *)mnl_attr_get_str(
- nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
open_json_object(NULL);
print_dev(rd, idx, name);
res_print_uint(rd, "pdn", pdn, nla_line[RDMA_NLDEV_ATTR_RES_PDN]);
diff --git a/rdma/res-qp.c b/rdma/res-qp.c
index a38be399..76a5334f 100644
--- a/rdma/res-qp.c
+++ b/rdma/res-qp.c
@@ -148,17 +148,16 @@ static int res_qp_line(struct rd *rd, const char *name, int idx,
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
+ } else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
+ comm = (char *)mnl_attr_get_str(
+ nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
}
if (rd_is_filtered_attr(rd, "pid", pid,
nla_line[RDMA_NLDEV_ATTR_RES_PID]))
goto out;
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
- /* discard const from mnl_attr_get_str */
- comm = (char *)mnl_attr_get_str(
- nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
open_json_object(NULL);
print_link(rd, idx, name, port, nla_line);
res_print_uint(rd, "lqpn", lqpn, nla_line[RDMA_NLDEV_ATTR_RES_LQPN]);
diff --git a/rdma/res-srq.c b/rdma/res-srq.c
index 3038c352..982e3fe9 100644
--- a/rdma/res-srq.c
+++ b/rdma/res-srq.c
@@ -176,7 +176,12 @@ static int res_srq_line(struct rd *rd, const char *name, int idx,
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
+ } else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
+ comm = (char *)mnl_attr_get_str(
+ nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
}
+
if (rd_is_filtered_attr(rd, "pid", pid,
nla_line[RDMA_NLDEV_ATTR_RES_PID]))
goto out;
@@ -209,11 +214,6 @@ static int res_srq_line(struct rd *rd, const char *name, int idx,
MNL_CB_OK)
goto out;
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME])
- /* discard const from mnl_attr_get_str */
- comm = (char *)mnl_attr_get_str(
- nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
-
open_json_object(NULL);
print_dev(rd, idx, name);
res_print_uint(rd, "srqn", srqn, nla_line[RDMA_NLDEV_ATTR_RES_SRQN]);
diff --git a/rdma/stat.c b/rdma/stat.c
index 66121b12..cf2c705b 100644
--- a/rdma/stat.c
+++ b/rdma/stat.c
@@ -256,19 +256,18 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) {
pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]);
comm = get_task_name(pid);
+ } else if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
+ /* discard const from mnl_attr_get_str */
+ comm = (char *)mnl_attr_get_str(
+ nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
}
+
if (rd_is_filtered_attr(rd, "pid", pid,
nla_line[RDMA_NLDEV_ATTR_RES_PID])) {
ret = MNL_CB_OK;
goto out;
}
- if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) {
- /* discard const from mnl_attr_get_str */
- comm = (char *)mnl_attr_get_str(
- nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]);
- }
-
mnl_attr_for_each_nested(nla_entry, qp_table) {
struct nlattr *qp_line[RDMA_NLDEV_ATTR_MAX] = {};
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread