* [PATCH iproute2 v2 0/2] fix memory leak in get_task_name() @ 2022-03-02 12:28 Andrea Claudi 2022-03-02 12:28 ` [PATCH iproute2 v2 1/2] lib/fs: " Andrea Claudi 2022-03-02 12:28 ` [PATCH iproute2 v2 2/2] rdma: make RES_PID and RES_KERN_NAME alternative to each other Andrea Claudi 0 siblings, 2 replies; 12+ messages in thread From: Andrea Claudi @ 2022-03-02 12:28 UTC (permalink / raw) To: netdev; +Cc: stephen, dsahern, markzhang, leonro This series fixes some memory leaks related to the usage of the get_task_name() function from lib/fs.c. Patch 2/2 addresses a coverity warning related to this memory leak, making the code a bit more readable by humans and coverity. Changelog: ---------- v1 -> v2 - on Stephen's suggestion, drop asprintf() and use a local var for path; additionally drop %m from fscanf to not allocate memory, and resort to a param to return task name to the caller. - patch 2/3 of the original series is no more needed. Andrea Claudi (2): lib/fs: fix memory leak in get_task_name() rdma: make RES_PID and RES_KERN_NAME alternative to each other include/utils.h | 2 +- ip/iptuntap.c | 17 ++++++++++------- lib/fs.c | 20 ++++++++++---------- rdma/res-cmid.c | 18 +++++++++--------- rdma/res-cq.c | 17 +++++++++-------- rdma/res-ctx.c | 16 ++++++++-------- rdma/res-mr.c | 15 ++++++++------- rdma/res-pd.c | 17 +++++++++-------- rdma/res-qp.c | 16 ++++++++-------- rdma/res-srq.c | 17 +++++++++-------- rdma/stat.c | 14 +++++++++----- 11 files changed, 90 insertions(+), 79 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-02 12:28 [PATCH iproute2 v2 0/2] fix memory leak in get_task_name() Andrea Claudi @ 2022-03-02 12:28 ` Andrea Claudi 2022-03-03 18:56 ` Leon Romanovsky 2022-03-07 17:58 ` David Ahern 2022-03-02 12:28 ` [PATCH iproute2 v2 2/2] rdma: make RES_PID and RES_KERN_NAME alternative to each other Andrea Claudi 1 sibling, 2 replies; 12+ messages in thread From: Andrea Claudi @ 2022-03-02 12:28 UTC (permalink / raw) To: netdev; +Cc: stephen, dsahern, markzhang, leonro asprintf() allocates memory which is not freed on the error path of get_task_name(), thus potentially leading to memory leaks. Rework get_task_name() and avoid asprintf() usage and memory allocation, returning the task string to the caller using an additional char* parameter. Fixes: 81bfd01a4c9e ("lib: move get_task_name() from rdma") Signed-off-by: Andrea Claudi <aclaudi@redhat.com> --- include/utils.h | 2 +- ip/iptuntap.c | 17 ++++++++++------- lib/fs.c | 20 ++++++++++---------- rdma/res-cmid.c | 8 +++++--- rdma/res-cq.c | 8 +++++--- rdma/res-ctx.c | 7 ++++--- rdma/res-mr.c | 7 ++++--- rdma/res-pd.c | 8 +++++--- rdma/res-qp.c | 7 ++++--- rdma/res-srq.c | 7 ++++--- rdma/stat.c | 5 ++++- 11 files changed, 56 insertions(+), 40 deletions(-) diff --git a/include/utils.h b/include/utils.h index b6c468e9..81294488 100644 --- a/include/utils.h +++ b/include/utils.h @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); __u64 get_cgroup2_id(const char *path); char *get_cgroup2_path(__u64 id, bool full); int get_command_name(const char *pid, char *comm, size_t len); -char *get_task_name(pid_t pid); +int get_task_name(pid_t pid, char *name); int get_rtnl_link_stats_rta(struct rtnl_link_stats64 *stats64, struct rtattr *tb[]); diff --git a/ip/iptuntap.c b/ip/iptuntap.c index 385d2bd8..2ae6b1a1 100644 --- a/ip/iptuntap.c +++ b/ip/iptuntap.c @@ -321,14 +321,17 @@ static void show_processes(const char *name) } else if (err == 2 && !strcmp("iff", key) && !strcmp(name, value)) { - char *pname = get_task_name(pid); - - print_string(PRINT_ANY, "name", - "%s", pname ? : "<NULL>"); + SPRINT_BUF(pname); + + if (get_task_name(pid, pname)) { + print_string(PRINT_ANY, "name", + "%s", "<NULL>"); + } else { + print_string(PRINT_ANY, "name", + "%s", pname); + } - print_uint(PRINT_ANY, "pid", - "(%d)", pid); - free(pname); + print_uint(PRINT_ANY, "pid", "(%d)", pid); } free(key); diff --git a/lib/fs.c b/lib/fs.c index f6f5f8a0..03df0f6a 100644 --- a/lib/fs.c +++ b/lib/fs.c @@ -342,25 +342,25 @@ int get_command_name(const char *pid, char *comm, size_t len) return 0; } -char *get_task_name(pid_t pid) +int get_task_name(pid_t pid, char *name) { - char *comm; + char path[PATH_MAX]; FILE *f; if (!pid) - return NULL; + return -1; - if (asprintf(&comm, "/proc/%d/comm", pid) < 0) - return NULL; + if (snprintf(path, sizeof(path), "/proc/%d/comm", pid) >= sizeof(path)) + return -1; - f = fopen(comm, "r"); + f = fopen(path, "r"); if (!f) - return NULL; + return -1; - if (fscanf(f, "%ms\n", &comm) != 1) - comm = NULL; + if (fscanf(f, "%s\n", name) != 1) + return -1; fclose(f); - return comm; + return 0; } diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c index bfaa47b5..3475349d 100644 --- a/rdma/res-cmid.c +++ b/rdma/res-cmid.c @@ -159,8 +159,11 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, goto out; if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { + SPRINT_BUF(b); + pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); - comm = get_task_name(pid); + if (!get_task_name(pid, b)) + comm = b; } if (rd_is_filtered_attr(rd, "pid", pid, @@ -199,8 +202,7 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); newline(rd); -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) - free(comm); +out: return MNL_CB_OK; } diff --git a/rdma/res-cq.c b/rdma/res-cq.c index 9e7c4f51..5ed455ea 100644 --- a/rdma/res-cq.c +++ b/rdma/res-cq.c @@ -84,8 +84,11 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, goto out; if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { + SPRINT_BUF(b); + pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); - comm = get_task_name(pid); + if (!get_task_name(pid, b)) + comm = b; } if (rd_is_filtered_attr(rd, "pid", pid, @@ -123,8 +126,7 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); newline(rd); -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) - free(comm); +out: return MNL_CB_OK; } diff --git a/rdma/res-ctx.c b/rdma/res-ctx.c index 30afe97a..fbd52dd5 100644 --- a/rdma/res-ctx.c +++ b/rdma/res-ctx.c @@ -18,8 +18,11 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, return MNL_CB_ERROR; if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { + SPRINT_BUF(b); + pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); - comm = get_task_name(pid); + if (!get_task_name(pid, b)) + comm = b; } if (rd_is_filtered_attr(rd, "pid", pid, @@ -48,8 +51,6 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, newline(rd); out: - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) - free(comm); return MNL_CB_OK; } diff --git a/rdma/res-mr.c b/rdma/res-mr.c index 1bf73f3a..6a59d9e4 100644 --- a/rdma/res-mr.c +++ b/rdma/res-mr.c @@ -47,8 +47,11 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, goto out; if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { + SPRINT_BUF(b); + pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); - comm = get_task_name(pid); + if (!get_task_name(pid, b)) + comm = b; } if (rd_is_filtered_attr(rd, "pid", pid, @@ -87,8 +90,6 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, newline(rd); out: - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) - free(comm); return MNL_CB_OK; } diff --git a/rdma/res-pd.c b/rdma/res-pd.c index df538010..a51bb634 100644 --- a/rdma/res-pd.c +++ b/rdma/res-pd.c @@ -34,8 +34,11 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, nla_line[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY]); if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { + SPRINT_BUF(b); + pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); - comm = get_task_name(pid); + if (!get_task_name(pid, b)) + comm = b; } if (rd_is_filtered_attr(rd, "pid", pid, @@ -76,8 +79,7 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); newline(rd); -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) - free(comm); +out: return MNL_CB_OK; } diff --git a/rdma/res-qp.c b/rdma/res-qp.c index a38be399..575e0529 100644 --- a/rdma/res-qp.c +++ b/rdma/res-qp.c @@ -146,8 +146,11 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, goto out; if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { + SPRINT_BUF(b); + pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); - comm = get_task_name(pid); + if (!get_task_name(pid, b)) + comm = b; } if (rd_is_filtered_attr(rd, "pid", pid, @@ -179,8 +182,6 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); newline(rd); out: - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) - free(comm); return MNL_CB_OK; } diff --git a/rdma/res-srq.c b/rdma/res-srq.c index 3038c352..945109fc 100644 --- a/rdma/res-srq.c +++ b/rdma/res-srq.c @@ -174,8 +174,11 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, return MNL_CB_ERROR; if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { + SPRINT_BUF(b); + pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); - comm = get_task_name(pid); + if (!get_task_name(pid, b)) + comm = b; } if (rd_is_filtered_attr(rd, "pid", pid, nla_line[RDMA_NLDEV_ATTR_RES_PID])) @@ -228,8 +231,6 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, newline(rd); out: - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) - free(comm); return MNL_CB_OK; } diff --git a/rdma/stat.c b/rdma/stat.c index adfcd34a..a63b70a4 100644 --- a/rdma/stat.c +++ b/rdma/stat.c @@ -248,8 +248,11 @@ static int res_counter_line(struct rd *rd, const char *name, int index, return MNL_CB_OK; if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { + SPRINT_BUF(b); + pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); - comm = get_task_name(pid); + if (!get_task_name(pid, b)) + comm = b; } if (rd_is_filtered_attr(rd, "pid", pid, nla_line[RDMA_NLDEV_ATTR_RES_PID])) -- 2.35.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-02 12:28 ` [PATCH iproute2 v2 1/2] lib/fs: " Andrea Claudi @ 2022-03-03 18:56 ` Leon Romanovsky 2022-03-07 18:07 ` Andrea Claudi 2022-03-07 17:58 ` David Ahern 1 sibling, 1 reply; 12+ messages in thread From: Leon Romanovsky @ 2022-03-03 18:56 UTC (permalink / raw) To: Andrea Claudi; +Cc: netdev, stephen, dsahern, markzhang On Wed, Mar 02, 2022 at 01:28:47PM +0100, Andrea Claudi wrote: > asprintf() allocates memory which is not freed on the error path of > get_task_name(), thus potentially leading to memory leaks. Not really, memory is released when the application exits, which is the case here. > > Rework get_task_name() and avoid asprintf() usage and memory allocation, > returning the task string to the caller using an additional char* > parameter. > > Fixes: 81bfd01a4c9e ("lib: move get_task_name() from rdma") As I was told before, in netdev Fixes means that it is a bug that affects users. This is not the case here. > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> > --- > include/utils.h | 2 +- > ip/iptuntap.c | 17 ++++++++++------- > lib/fs.c | 20 ++++++++++---------- > rdma/res-cmid.c | 8 +++++--- > rdma/res-cq.c | 8 +++++--- > rdma/res-ctx.c | 7 ++++--- > rdma/res-mr.c | 7 ++++--- > rdma/res-pd.c | 8 +++++--- > rdma/res-qp.c | 7 ++++--- > rdma/res-srq.c | 7 ++++--- > rdma/stat.c | 5 ++++- > 11 files changed, 56 insertions(+), 40 deletions(-) Honestly, I don't see any need in both patches. Thanks > > diff --git a/include/utils.h b/include/utils.h > index b6c468e9..81294488 100644 > --- a/include/utils.h > +++ b/include/utils.h > @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); > __u64 get_cgroup2_id(const char *path); > char *get_cgroup2_path(__u64 id, bool full); > int get_command_name(const char *pid, char *comm, size_t len); > -char *get_task_name(pid_t pid); > +int get_task_name(pid_t pid, char *name); > > int get_rtnl_link_stats_rta(struct rtnl_link_stats64 *stats64, > struct rtattr *tb[]); > diff --git a/ip/iptuntap.c b/ip/iptuntap.c > index 385d2bd8..2ae6b1a1 100644 > --- a/ip/iptuntap.c > +++ b/ip/iptuntap.c > @@ -321,14 +321,17 @@ static void show_processes(const char *name) > } else if (err == 2 && > !strcmp("iff", key) && > !strcmp(name, value)) { > - char *pname = get_task_name(pid); > - > - print_string(PRINT_ANY, "name", > - "%s", pname ? : "<NULL>"); > + SPRINT_BUF(pname); > + > + if (get_task_name(pid, pname)) { > + print_string(PRINT_ANY, "name", > + "%s", "<NULL>"); > + } else { > + print_string(PRINT_ANY, "name", > + "%s", pname); > + } > > - print_uint(PRINT_ANY, "pid", > - "(%d)", pid); > - free(pname); > + print_uint(PRINT_ANY, "pid", "(%d)", pid); > } > > free(key); > diff --git a/lib/fs.c b/lib/fs.c > index f6f5f8a0..03df0f6a 100644 > --- a/lib/fs.c > +++ b/lib/fs.c > @@ -342,25 +342,25 @@ int get_command_name(const char *pid, char *comm, size_t len) > return 0; > } > > -char *get_task_name(pid_t pid) > +int get_task_name(pid_t pid, char *name) > { > - char *comm; > + char path[PATH_MAX]; > FILE *f; > > if (!pid) > - return NULL; > + return -1; > > - if (asprintf(&comm, "/proc/%d/comm", pid) < 0) > - return NULL; > + if (snprintf(path, sizeof(path), "/proc/%d/comm", pid) >= sizeof(path)) > + return -1; > > - f = fopen(comm, "r"); > + f = fopen(path, "r"); > if (!f) > - return NULL; > + return -1; > > - if (fscanf(f, "%ms\n", &comm) != 1) > - comm = NULL; > + if (fscanf(f, "%s\n", name) != 1) > + return -1; > > fclose(f); > > - return comm; > + return 0; > } > diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c > index bfaa47b5..3475349d 100644 > --- a/rdma/res-cmid.c > +++ b/rdma/res-cmid.c > @@ -159,8 +159,11 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, > goto out; > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > + SPRINT_BUF(b); > + > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > - comm = get_task_name(pid); > + if (!get_task_name(pid, b)) > + comm = b; > } > > if (rd_is_filtered_attr(rd, "pid", pid, > @@ -199,8 +202,7 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > newline(rd); > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > - free(comm); > +out: > return MNL_CB_OK; > } > > diff --git a/rdma/res-cq.c b/rdma/res-cq.c > index 9e7c4f51..5ed455ea 100644 > --- a/rdma/res-cq.c > +++ b/rdma/res-cq.c > @@ -84,8 +84,11 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, > goto out; > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > + SPRINT_BUF(b); > + > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > - comm = get_task_name(pid); > + if (!get_task_name(pid, b)) > + comm = b; > } > > if (rd_is_filtered_attr(rd, "pid", pid, > @@ -123,8 +126,7 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > newline(rd); > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > - free(comm); > +out: > return MNL_CB_OK; > } > > diff --git a/rdma/res-ctx.c b/rdma/res-ctx.c > index 30afe97a..fbd52dd5 100644 > --- a/rdma/res-ctx.c > +++ b/rdma/res-ctx.c > @@ -18,8 +18,11 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, > return MNL_CB_ERROR; > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > + SPRINT_BUF(b); > + > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > - comm = get_task_name(pid); > + if (!get_task_name(pid, b)) > + comm = b; > } > > if (rd_is_filtered_attr(rd, "pid", pid, > @@ -48,8 +51,6 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, > newline(rd); > > out: > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > - free(comm); > return MNL_CB_OK; > } > > diff --git a/rdma/res-mr.c b/rdma/res-mr.c > index 1bf73f3a..6a59d9e4 100644 > --- a/rdma/res-mr.c > +++ b/rdma/res-mr.c > @@ -47,8 +47,11 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, > goto out; > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > + SPRINT_BUF(b); > + > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > - comm = get_task_name(pid); > + if (!get_task_name(pid, b)) > + comm = b; > } > > if (rd_is_filtered_attr(rd, "pid", pid, > @@ -87,8 +90,6 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, > newline(rd); > > out: > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > - free(comm); > return MNL_CB_OK; > } > > diff --git a/rdma/res-pd.c b/rdma/res-pd.c > index df538010..a51bb634 100644 > --- a/rdma/res-pd.c > +++ b/rdma/res-pd.c > @@ -34,8 +34,11 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, > nla_line[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY]); > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > + SPRINT_BUF(b); > + > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > - comm = get_task_name(pid); > + if (!get_task_name(pid, b)) > + comm = b; > } > > if (rd_is_filtered_attr(rd, "pid", pid, > @@ -76,8 +79,7 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > newline(rd); > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > - free(comm); > +out: > return MNL_CB_OK; > } > > diff --git a/rdma/res-qp.c b/rdma/res-qp.c > index a38be399..575e0529 100644 > --- a/rdma/res-qp.c > +++ b/rdma/res-qp.c > @@ -146,8 +146,11 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, > goto out; > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > + SPRINT_BUF(b); > + > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > - comm = get_task_name(pid); > + if (!get_task_name(pid, b)) > + comm = b; > } > > if (rd_is_filtered_attr(rd, "pid", pid, > @@ -179,8 +182,6 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > newline(rd); > out: > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > - free(comm); > return MNL_CB_OK; > } > > diff --git a/rdma/res-srq.c b/rdma/res-srq.c > index 3038c352..945109fc 100644 > --- a/rdma/res-srq.c > +++ b/rdma/res-srq.c > @@ -174,8 +174,11 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, > return MNL_CB_ERROR; > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > + SPRINT_BUF(b); > + > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > - comm = get_task_name(pid); > + if (!get_task_name(pid, b)) > + comm = b; > } > if (rd_is_filtered_attr(rd, "pid", pid, > nla_line[RDMA_NLDEV_ATTR_RES_PID])) > @@ -228,8 +231,6 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, > newline(rd); > > out: > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > - free(comm); > return MNL_CB_OK; > } > > diff --git a/rdma/stat.c b/rdma/stat.c > index adfcd34a..a63b70a4 100644 > --- a/rdma/stat.c > +++ b/rdma/stat.c > @@ -248,8 +248,11 @@ static int res_counter_line(struct rd *rd, const char *name, int index, > return MNL_CB_OK; > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > + SPRINT_BUF(b); > + > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > - comm = get_task_name(pid); > + if (!get_task_name(pid, b)) > + comm = b; > } > if (rd_is_filtered_attr(rd, "pid", pid, > nla_line[RDMA_NLDEV_ATTR_RES_PID])) > -- > 2.35.1 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-03 18:56 ` Leon Romanovsky @ 2022-03-07 18:07 ` Andrea Claudi 2022-03-07 18:12 ` David Ahern 2022-03-09 13:39 ` Leon Romanovsky 0 siblings, 2 replies; 12+ messages in thread From: Andrea Claudi @ 2022-03-07 18:07 UTC (permalink / raw) To: Leon Romanovsky; +Cc: netdev, stephen, dsahern, markzhang Hi Leon, Thanks for your review. On Thu, Mar 03, 2022 at 08:56:57PM +0200, Leon Romanovsky wrote: > On Wed, Mar 02, 2022 at 01:28:47PM +0100, Andrea Claudi wrote: > > asprintf() allocates memory which is not freed on the error path of > > get_task_name(), thus potentially leading to memory leaks. > > Not really, memory is released when the application exits, which is the > case here. > That's certainly true. However this is still a leak in the time frame between get_task_name function call and the application exit. For example: $ ip -d -b - << EOF tuntap show monitor EOF calls get_task_name one or more time (once for each tun interface), and leaks memory indefinitely, if ip is not interrupted in some way. Of course this is a corner case, and the leaks should anyway be small. However I cannot see this as a good reason not to fix it. > > > > Rework get_task_name() and avoid asprintf() usage and memory allocation, > > returning the task string to the caller using an additional char* > > parameter. > > > > Fixes: 81bfd01a4c9e ("lib: move get_task_name() from rdma") > > As I was told before, in netdev Fixes means that it is a bug that > affects users. This is not the case here. Thanks for letting me know. I usually rely a lot on Fixes: as iproute2 package maintainer, but I'll change my habits if this is the common understanding. Stephen, David, WDYT? > > > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> > > --- > > include/utils.h | 2 +- > > ip/iptuntap.c | 17 ++++++++++------- > > lib/fs.c | 20 ++++++++++---------- > > rdma/res-cmid.c | 8 +++++--- > > rdma/res-cq.c | 8 +++++--- > > rdma/res-ctx.c | 7 ++++--- > > rdma/res-mr.c | 7 ++++--- > > rdma/res-pd.c | 8 +++++--- > > rdma/res-qp.c | 7 ++++--- > > rdma/res-srq.c | 7 ++++--- > > rdma/stat.c | 5 ++++- > > 11 files changed, 56 insertions(+), 40 deletions(-) > > Honestly, I don't see any need in both patches. > > Thanks > > > > > diff --git a/include/utils.h b/include/utils.h > > index b6c468e9..81294488 100644 > > --- a/include/utils.h > > +++ b/include/utils.h > > @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); > > __u64 get_cgroup2_id(const char *path); > > char *get_cgroup2_path(__u64 id, bool full); > > int get_command_name(const char *pid, char *comm, size_t len); > > -char *get_task_name(pid_t pid); > > +int get_task_name(pid_t pid, char *name); > > > > int get_rtnl_link_stats_rta(struct rtnl_link_stats64 *stats64, > > struct rtattr *tb[]); > > diff --git a/ip/iptuntap.c b/ip/iptuntap.c > > index 385d2bd8..2ae6b1a1 100644 > > --- a/ip/iptuntap.c > > +++ b/ip/iptuntap.c > > @@ -321,14 +321,17 @@ static void show_processes(const char *name) > > } else if (err == 2 && > > !strcmp("iff", key) && > > !strcmp(name, value)) { > > - char *pname = get_task_name(pid); > > - > > - print_string(PRINT_ANY, "name", > > - "%s", pname ? : "<NULL>"); > > + SPRINT_BUF(pname); > > + > > + if (get_task_name(pid, pname)) { > > + print_string(PRINT_ANY, "name", > > + "%s", "<NULL>"); > > + } else { > > + print_string(PRINT_ANY, "name", > > + "%s", pname); > > + } > > > > - print_uint(PRINT_ANY, "pid", > > - "(%d)", pid); > > - free(pname); > > + print_uint(PRINT_ANY, "pid", "(%d)", pid); > > } > > > > free(key); > > diff --git a/lib/fs.c b/lib/fs.c > > index f6f5f8a0..03df0f6a 100644 > > --- a/lib/fs.c > > +++ b/lib/fs.c > > @@ -342,25 +342,25 @@ int get_command_name(const char *pid, char *comm, size_t len) > > return 0; > > } > > > > -char *get_task_name(pid_t pid) > > +int get_task_name(pid_t pid, char *name) > > { > > - char *comm; > > + char path[PATH_MAX]; > > FILE *f; > > > > if (!pid) > > - return NULL; > > + return -1; > > > > - if (asprintf(&comm, "/proc/%d/comm", pid) < 0) > > - return NULL; > > + if (snprintf(path, sizeof(path), "/proc/%d/comm", pid) >= sizeof(path)) > > + return -1; > > > > - f = fopen(comm, "r"); > > + f = fopen(path, "r"); > > if (!f) > > - return NULL; > > + return -1; > > > > - if (fscanf(f, "%ms\n", &comm) != 1) > > - comm = NULL; > > + if (fscanf(f, "%s\n", name) != 1) > > + return -1; > > > > fclose(f); > > > > - return comm; > > + return 0; > > } > > diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c > > index bfaa47b5..3475349d 100644 > > --- a/rdma/res-cmid.c > > +++ b/rdma/res-cmid.c > > @@ -159,8 +159,11 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, > > goto out; > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > + SPRINT_BUF(b); > > + > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > - comm = get_task_name(pid); > > + if (!get_task_name(pid, b)) > > + comm = b; > > } > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > @@ -199,8 +202,7 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, > > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > > newline(rd); > > > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > - free(comm); > > +out: > > return MNL_CB_OK; > > } > > > > diff --git a/rdma/res-cq.c b/rdma/res-cq.c > > index 9e7c4f51..5ed455ea 100644 > > --- a/rdma/res-cq.c > > +++ b/rdma/res-cq.c > > @@ -84,8 +84,11 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, > > goto out; > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > + SPRINT_BUF(b); > > + > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > - comm = get_task_name(pid); > > + if (!get_task_name(pid, b)) > > + comm = b; > > } > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > @@ -123,8 +126,7 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, > > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > > newline(rd); > > > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > - free(comm); > > +out: > > return MNL_CB_OK; > > } > > > > diff --git a/rdma/res-ctx.c b/rdma/res-ctx.c > > index 30afe97a..fbd52dd5 100644 > > --- a/rdma/res-ctx.c > > +++ b/rdma/res-ctx.c > > @@ -18,8 +18,11 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, > > return MNL_CB_ERROR; > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > + SPRINT_BUF(b); > > + > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > - comm = get_task_name(pid); > > + if (!get_task_name(pid, b)) > > + comm = b; > > } > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > @@ -48,8 +51,6 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, > > newline(rd); > > > > out: > > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > - free(comm); > > return MNL_CB_OK; > > } > > > > diff --git a/rdma/res-mr.c b/rdma/res-mr.c > > index 1bf73f3a..6a59d9e4 100644 > > --- a/rdma/res-mr.c > > +++ b/rdma/res-mr.c > > @@ -47,8 +47,11 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, > > goto out; > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > + SPRINT_BUF(b); > > + > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > - comm = get_task_name(pid); > > + if (!get_task_name(pid, b)) > > + comm = b; > > } > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > @@ -87,8 +90,6 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, > > newline(rd); > > > > out: > > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > - free(comm); > > return MNL_CB_OK; > > } > > > > diff --git a/rdma/res-pd.c b/rdma/res-pd.c > > index df538010..a51bb634 100644 > > --- a/rdma/res-pd.c > > +++ b/rdma/res-pd.c > > @@ -34,8 +34,11 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, > > nla_line[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY]); > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > + SPRINT_BUF(b); > > + > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > - comm = get_task_name(pid); > > + if (!get_task_name(pid, b)) > > + comm = b; > > } > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > @@ -76,8 +79,7 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, > > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > > newline(rd); > > > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > - free(comm); > > +out: > > return MNL_CB_OK; > > } > > > > diff --git a/rdma/res-qp.c b/rdma/res-qp.c > > index a38be399..575e0529 100644 > > --- a/rdma/res-qp.c > > +++ b/rdma/res-qp.c > > @@ -146,8 +146,11 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, > > goto out; > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > + SPRINT_BUF(b); > > + > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > - comm = get_task_name(pid); > > + if (!get_task_name(pid, b)) > > + comm = b; > > } > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > @@ -179,8 +182,6 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, > > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > > newline(rd); > > out: > > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > - free(comm); > > return MNL_CB_OK; > > } > > > > diff --git a/rdma/res-srq.c b/rdma/res-srq.c > > index 3038c352..945109fc 100644 > > --- a/rdma/res-srq.c > > +++ b/rdma/res-srq.c > > @@ -174,8 +174,11 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, > > return MNL_CB_ERROR; > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > + SPRINT_BUF(b); > > + > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > - comm = get_task_name(pid); > > + if (!get_task_name(pid, b)) > > + comm = b; > > } > > if (rd_is_filtered_attr(rd, "pid", pid, > > nla_line[RDMA_NLDEV_ATTR_RES_PID])) > > @@ -228,8 +231,6 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, > > newline(rd); > > > > out: > > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > - free(comm); > > return MNL_CB_OK; > > } > > > > diff --git a/rdma/stat.c b/rdma/stat.c > > index adfcd34a..a63b70a4 100644 > > --- a/rdma/stat.c > > +++ b/rdma/stat.c > > @@ -248,8 +248,11 @@ static int res_counter_line(struct rd *rd, const char *name, int index, > > return MNL_CB_OK; > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > + SPRINT_BUF(b); > > + > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > - comm = get_task_name(pid); > > + if (!get_task_name(pid, b)) > > + comm = b; > > } > > if (rd_is_filtered_attr(rd, "pid", pid, > > nla_line[RDMA_NLDEV_ATTR_RES_PID])) > > -- > > 2.35.1 > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-07 18:07 ` Andrea Claudi @ 2022-03-07 18:12 ` David Ahern 2022-03-09 13:39 ` Leon Romanovsky 1 sibling, 0 replies; 12+ messages in thread From: David Ahern @ 2022-03-07 18:12 UTC (permalink / raw) To: Andrea Claudi, Leon Romanovsky; +Cc: netdev, stephen, markzhang On 3/7/22 11:07 AM, Andrea Claudi wrote: > Thanks for letting me know. I usually rely a lot on Fixes: as iproute2 > package maintainer, but I'll change my habits if this is the common > understanding. Stephen, David, WDYT? I think a Fixes tag is relevant here. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-07 18:07 ` Andrea Claudi 2022-03-07 18:12 ` David Ahern @ 2022-03-09 13:39 ` Leon Romanovsky 1 sibling, 0 replies; 12+ messages in thread From: Leon Romanovsky @ 2022-03-09 13:39 UTC (permalink / raw) To: Andrea Claudi; +Cc: netdev, stephen, dsahern, markzhang On Mon, Mar 07, 2022 at 07:07:10PM +0100, Andrea Claudi wrote: > Hi Leon, > Thanks for your review. > > On Thu, Mar 03, 2022 at 08:56:57PM +0200, Leon Romanovsky wrote: > > On Wed, Mar 02, 2022 at 01:28:47PM +0100, Andrea Claudi wrote: > > > asprintf() allocates memory which is not freed on the error path of > > > get_task_name(), thus potentially leading to memory leaks. > > > > Not really, memory is released when the application exits, which is the > > case here. > > > > That's certainly true. However this is still a leak in the time frame > between get_task_name function call and the application exit. For > example: > > $ ip -d -b - << EOF > tuntap show > monitor > EOF > > calls get_task_name one or more time (once for each tun interface), and > leaks memory indefinitely, if ip is not interrupted in some way. > > Of course this is a corner case, and the leaks should anyway be small. > However I cannot see this as a good reason not to fix it. > > > > > > > Rework get_task_name() and avoid asprintf() usage and memory allocation, > > > returning the task string to the caller using an additional char* > > > parameter. > > > > > > Fixes: 81bfd01a4c9e ("lib: move get_task_name() from rdma") > > > > As I was told before, in netdev Fixes means that it is a bug that > > affects users. This is not the case here. > > Thanks for letting me know. I usually rely a lot on Fixes: as iproute2 > package maintainer, but I'll change my habits if this is the common > understanding. Stephen, David, WDYT? > > > > > > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> > > > --- > > > include/utils.h | 2 +- > > > ip/iptuntap.c | 17 ++++++++++------- Ahh, sorry, I missed this user. So yes, Fixes is needed here. Thanks > > > lib/fs.c | 20 ++++++++++---------- > > > rdma/res-cmid.c | 8 +++++--- > > > rdma/res-cq.c | 8 +++++--- > > > rdma/res-ctx.c | 7 ++++--- > > > rdma/res-mr.c | 7 ++++--- > > > rdma/res-pd.c | 8 +++++--- > > > rdma/res-qp.c | 7 ++++--- > > > rdma/res-srq.c | 7 ++++--- > > > rdma/stat.c | 5 ++++- > > > 11 files changed, 56 insertions(+), 40 deletions(-) > > > > Honestly, I don't see any need in both patches. > > > > Thanks > > > > > > > > diff --git a/include/utils.h b/include/utils.h > > > index b6c468e9..81294488 100644 > > > --- a/include/utils.h > > > +++ b/include/utils.h > > > @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); > > > __u64 get_cgroup2_id(const char *path); > > > char *get_cgroup2_path(__u64 id, bool full); > > > int get_command_name(const char *pid, char *comm, size_t len); > > > -char *get_task_name(pid_t pid); > > > +int get_task_name(pid_t pid, char *name); > > > > > > int get_rtnl_link_stats_rta(struct rtnl_link_stats64 *stats64, > > > struct rtattr *tb[]); > > > diff --git a/ip/iptuntap.c b/ip/iptuntap.c > > > index 385d2bd8..2ae6b1a1 100644 > > > --- a/ip/iptuntap.c > > > +++ b/ip/iptuntap.c > > > @@ -321,14 +321,17 @@ static void show_processes(const char *name) > > > } else if (err == 2 && > > > !strcmp("iff", key) && > > > !strcmp(name, value)) { > > > - char *pname = get_task_name(pid); > > > - > > > - print_string(PRINT_ANY, "name", > > > - "%s", pname ? : "<NULL>"); > > > + SPRINT_BUF(pname); > > > + > > > + if (get_task_name(pid, pname)) { > > > + print_string(PRINT_ANY, "name", > > > + "%s", "<NULL>"); > > > + } else { > > > + print_string(PRINT_ANY, "name", > > > + "%s", pname); > > > + } > > > > > > - print_uint(PRINT_ANY, "pid", > > > - "(%d)", pid); > > > - free(pname); > > > + print_uint(PRINT_ANY, "pid", "(%d)", pid); > > > } > > > > > > free(key); > > > diff --git a/lib/fs.c b/lib/fs.c > > > index f6f5f8a0..03df0f6a 100644 > > > --- a/lib/fs.c > > > +++ b/lib/fs.c > > > @@ -342,25 +342,25 @@ int get_command_name(const char *pid, char *comm, size_t len) > > > return 0; > > > } > > > > > > -char *get_task_name(pid_t pid) > > > +int get_task_name(pid_t pid, char *name) > > > { > > > - char *comm; > > > + char path[PATH_MAX]; > > > FILE *f; > > > > > > if (!pid) > > > - return NULL; > > > + return -1; > > > > > > - if (asprintf(&comm, "/proc/%d/comm", pid) < 0) > > > - return NULL; > > > + if (snprintf(path, sizeof(path), "/proc/%d/comm", pid) >= sizeof(path)) > > > + return -1; > > > > > > - f = fopen(comm, "r"); > > > + f = fopen(path, "r"); > > > if (!f) > > > - return NULL; > > > + return -1; > > > > > > - if (fscanf(f, "%ms\n", &comm) != 1) > > > - comm = NULL; > > > + if (fscanf(f, "%s\n", name) != 1) > > > + return -1; > > > > > > fclose(f); > > > > > > - return comm; > > > + return 0; > > > } > > > diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c > > > index bfaa47b5..3475349d 100644 > > > --- a/rdma/res-cmid.c > > > +++ b/rdma/res-cmid.c > > > @@ -159,8 +159,11 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, > > > goto out; > > > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > > + SPRINT_BUF(b); > > > + > > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > > - comm = get_task_name(pid); > > > + if (!get_task_name(pid, b)) > > > + comm = b; > > > } > > > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > > @@ -199,8 +202,7 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, > > > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > > > newline(rd); > > > > > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > > - free(comm); > > > +out: > > > return MNL_CB_OK; > > > } > > > > > > diff --git a/rdma/res-cq.c b/rdma/res-cq.c > > > index 9e7c4f51..5ed455ea 100644 > > > --- a/rdma/res-cq.c > > > +++ b/rdma/res-cq.c > > > @@ -84,8 +84,11 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, > > > goto out; > > > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > > + SPRINT_BUF(b); > > > + > > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > > - comm = get_task_name(pid); > > > + if (!get_task_name(pid, b)) > > > + comm = b; > > > } > > > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > > @@ -123,8 +126,7 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, > > > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > > > newline(rd); > > > > > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > > - free(comm); > > > +out: > > > return MNL_CB_OK; > > > } > > > > > > diff --git a/rdma/res-ctx.c b/rdma/res-ctx.c > > > index 30afe97a..fbd52dd5 100644 > > > --- a/rdma/res-ctx.c > > > +++ b/rdma/res-ctx.c > > > @@ -18,8 +18,11 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, > > > return MNL_CB_ERROR; > > > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > > + SPRINT_BUF(b); > > > + > > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > > - comm = get_task_name(pid); > > > + if (!get_task_name(pid, b)) > > > + comm = b; > > > } > > > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > > @@ -48,8 +51,6 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, > > > newline(rd); > > > > > > out: > > > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > > - free(comm); > > > return MNL_CB_OK; > > > } > > > > > > diff --git a/rdma/res-mr.c b/rdma/res-mr.c > > > index 1bf73f3a..6a59d9e4 100644 > > > --- a/rdma/res-mr.c > > > +++ b/rdma/res-mr.c > > > @@ -47,8 +47,11 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, > > > goto out; > > > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > > + SPRINT_BUF(b); > > > + > > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > > - comm = get_task_name(pid); > > > + if (!get_task_name(pid, b)) > > > + comm = b; > > > } > > > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > > @@ -87,8 +90,6 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, > > > newline(rd); > > > > > > out: > > > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > > - free(comm); > > > return MNL_CB_OK; > > > } > > > > > > diff --git a/rdma/res-pd.c b/rdma/res-pd.c > > > index df538010..a51bb634 100644 > > > --- a/rdma/res-pd.c > > > +++ b/rdma/res-pd.c > > > @@ -34,8 +34,11 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, > > > nla_line[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY]); > > > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > > + SPRINT_BUF(b); > > > + > > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > > - comm = get_task_name(pid); > > > + if (!get_task_name(pid, b)) > > > + comm = b; > > > } > > > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > > @@ -76,8 +79,7 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, > > > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > > > newline(rd); > > > > > > -out: if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > > - free(comm); > > > +out: > > > return MNL_CB_OK; > > > } > > > > > > diff --git a/rdma/res-qp.c b/rdma/res-qp.c > > > index a38be399..575e0529 100644 > > > --- a/rdma/res-qp.c > > > +++ b/rdma/res-qp.c > > > @@ -146,8 +146,11 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, > > > goto out; > > > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > > + SPRINT_BUF(b); > > > + > > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > > - comm = get_task_name(pid); > > > + if (!get_task_name(pid, b)) > > > + comm = b; > > > } > > > > > > if (rd_is_filtered_attr(rd, "pid", pid, > > > @@ -179,8 +182,6 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, > > > print_driver_table(rd, nla_line[RDMA_NLDEV_ATTR_DRIVER]); > > > newline(rd); > > > out: > > > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > > - free(comm); > > > return MNL_CB_OK; > > > } > > > > > > diff --git a/rdma/res-srq.c b/rdma/res-srq.c > > > index 3038c352..945109fc 100644 > > > --- a/rdma/res-srq.c > > > +++ b/rdma/res-srq.c > > > @@ -174,8 +174,11 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, > > > return MNL_CB_ERROR; > > > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > > + SPRINT_BUF(b); > > > + > > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > > - comm = get_task_name(pid); > > > + if (!get_task_name(pid, b)) > > > + comm = b; > > > } > > > if (rd_is_filtered_attr(rd, "pid", pid, > > > nla_line[RDMA_NLDEV_ATTR_RES_PID])) > > > @@ -228,8 +231,6 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, > > > newline(rd); > > > > > > out: > > > - if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) > > > - free(comm); > > > return MNL_CB_OK; > > > } > > > > > > diff --git a/rdma/stat.c b/rdma/stat.c > > > index adfcd34a..a63b70a4 100644 > > > --- a/rdma/stat.c > > > +++ b/rdma/stat.c > > > @@ -248,8 +248,11 @@ static int res_counter_line(struct rd *rd, const char *name, int index, > > > return MNL_CB_OK; > > > > > > if (nla_line[RDMA_NLDEV_ATTR_RES_PID]) { > > > + SPRINT_BUF(b); > > > + > > > pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); > > > - comm = get_task_name(pid); > > > + if (!get_task_name(pid, b)) > > > + comm = b; > > > } > > > if (rd_is_filtered_attr(rd, "pid", pid, > > > nla_line[RDMA_NLDEV_ATTR_RES_PID])) > > > -- > > > 2.35.1 > > > > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-02 12:28 ` [PATCH iproute2 v2 1/2] lib/fs: " Andrea Claudi 2022-03-03 18:56 ` Leon Romanovsky @ 2022-03-07 17:58 ` David Ahern 2022-03-07 18:14 ` Stephen Hemminger 2022-03-07 18:21 ` Andrea Claudi 1 sibling, 2 replies; 12+ messages in thread From: David Ahern @ 2022-03-07 17:58 UTC (permalink / raw) To: Andrea Claudi, netdev; +Cc: stephen, markzhang, leonro On 3/2/22 5:28 AM, Andrea Claudi wrote: > diff --git a/include/utils.h b/include/utils.h > index b6c468e9..81294488 100644 > --- a/include/utils.h > +++ b/include/utils.h > @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); > __u64 get_cgroup2_id(const char *path); > char *get_cgroup2_path(__u64 id, bool full); > int get_command_name(const char *pid, char *comm, size_t len); > -char *get_task_name(pid_t pid); > +int get_task_name(pid_t pid, char *name); > changing to an API with an assumed length is not better than the current situation. Why not just fixup the callers as needed to free the allocation? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-07 17:58 ` David Ahern @ 2022-03-07 18:14 ` Stephen Hemminger 2022-03-07 18:21 ` Andrea Claudi 1 sibling, 0 replies; 12+ messages in thread From: Stephen Hemminger @ 2022-03-07 18:14 UTC (permalink / raw) To: David Ahern; +Cc: Andrea Claudi, netdev, markzhang, leonro On Mon, 7 Mar 2022 10:58:37 -0700 David Ahern <dsahern@kernel.org> wrote: > On 3/2/22 5:28 AM, Andrea Claudi wrote: > > diff --git a/include/utils.h b/include/utils.h > > index b6c468e9..81294488 100644 > > --- a/include/utils.h > > +++ b/include/utils.h > > @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); > > __u64 get_cgroup2_id(const char *path); > > char *get_cgroup2_path(__u64 id, bool full); > > int get_command_name(const char *pid, char *comm, size_t len); > > -char *get_task_name(pid_t pid); > > +int get_task_name(pid_t pid, char *name); > > > > changing to an API with an assumed length is not better than the current > situation. Why not just fixup the callers as needed to free the allocation? I wonder if iproute2 should start using GCC attribute cleanup function (like systemd) or would this be too much of using a new thing? Not sure if using the attribute (wrapped in macro) just hides the problem ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-07 17:58 ` David Ahern 2022-03-07 18:14 ` Stephen Hemminger @ 2022-03-07 18:21 ` Andrea Claudi 2022-03-07 19:57 ` Stephen Hemminger 2022-03-07 22:38 ` David Ahern 1 sibling, 2 replies; 12+ messages in thread From: Andrea Claudi @ 2022-03-07 18:21 UTC (permalink / raw) To: David Ahern; +Cc: netdev, stephen, markzhang, leonro On Mon, Mar 07, 2022 at 10:58:37AM -0700, David Ahern wrote: > On 3/2/22 5:28 AM, Andrea Claudi wrote: > > diff --git a/include/utils.h b/include/utils.h > > index b6c468e9..81294488 100644 > > --- a/include/utils.h > > +++ b/include/utils.h > > @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); > > __u64 get_cgroup2_id(const char *path); > > char *get_cgroup2_path(__u64 id, bool full); > > int get_command_name(const char *pid, char *comm, size_t len); > > -char *get_task_name(pid_t pid); > > +int get_task_name(pid_t pid, char *name); > > > > changing to an API with an assumed length is not better than the current > situation. Why not just fixup the callers as needed to free the allocation? > I actually did that on v1. After Stephen's comment about asprintf(), I got the idea to make get_task_name() similar to get_command_name() and a bit more "user-friendly", so that callers do not need a free(). If you think this is not ideal, I can post a v3 with the necessary fixes to the callers. Thanks, Andrea ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-07 18:21 ` Andrea Claudi @ 2022-03-07 19:57 ` Stephen Hemminger 2022-03-07 22:38 ` David Ahern 1 sibling, 0 replies; 12+ messages in thread From: Stephen Hemminger @ 2022-03-07 19:57 UTC (permalink / raw) To: Andrea Claudi; +Cc: David Ahern, netdev, markzhang, leonro On Mon, 7 Mar 2022 19:21:41 +0100 Andrea Claudi <aclaudi@redhat.com> wrote: > On Mon, Mar 07, 2022 at 10:58:37AM -0700, David Ahern wrote: > > On 3/2/22 5:28 AM, Andrea Claudi wrote: > > > diff --git a/include/utils.h b/include/utils.h > > > index b6c468e9..81294488 100644 > > > --- a/include/utils.h > > > +++ b/include/utils.h > > > @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); > > > __u64 get_cgroup2_id(const char *path); > > > char *get_cgroup2_path(__u64 id, bool full); > > > int get_command_name(const char *pid, char *comm, size_t len); > > > -char *get_task_name(pid_t pid); > > > +int get_task_name(pid_t pid, char *name); > > > > > > > changing to an API with an assumed length is not better than the current > > situation. Why not just fixup the callers as needed to free the allocation? > > > > I actually did that on v1. After Stephen's comment about asprintf(), I > got the idea to make get_task_name() similar to get_command_name() and > a bit more "user-friendly", so that callers do not need a free(). > > If you think this is not ideal, I can post a v3 with the necessary fixes > to the callers. > > Thanks, > Andrea > My comment was purely a suggestion not a requirement. I have just had issues with complaints from compiler about code not checking return value from asprintf, so tend to avoid it. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2 v2 1/2] lib/fs: fix memory leak in get_task_name() 2022-03-07 18:21 ` Andrea Claudi 2022-03-07 19:57 ` Stephen Hemminger @ 2022-03-07 22:38 ` David Ahern 1 sibling, 0 replies; 12+ messages in thread From: David Ahern @ 2022-03-07 22:38 UTC (permalink / raw) To: Andrea Claudi; +Cc: netdev, stephen, markzhang, leonro On 3/7/22 11:21 AM, Andrea Claudi wrote: > On Mon, Mar 07, 2022 at 10:58:37AM -0700, David Ahern wrote: >> On 3/2/22 5:28 AM, Andrea Claudi wrote: >>> diff --git a/include/utils.h b/include/utils.h >>> index b6c468e9..81294488 100644 >>> --- a/include/utils.h >>> +++ b/include/utils.h >>> @@ -307,7 +307,7 @@ char *find_cgroup2_mount(bool do_mount); >>> __u64 get_cgroup2_id(const char *path); >>> char *get_cgroup2_path(__u64 id, bool full); >>> int get_command_name(const char *pid, char *comm, size_t len); >>> -char *get_task_name(pid_t pid); >>> +int get_task_name(pid_t pid, char *name); >>> >> >> changing to an API with an assumed length is not better than the current >> situation. Why not just fixup the callers as needed to free the allocation? >> > > I actually did that on v1. After Stephen's comment about asprintf(), I > got the idea to make get_task_name() similar to get_command_name() and > a bit more "user-friendly", so that callers do not need a free(). > get_command_name passes a buffer and length. That's a better API - no assumptions. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH iproute2 v2 2/2] rdma: make RES_PID and RES_KERN_NAME alternative to each other 2022-03-02 12:28 [PATCH iproute2 v2 0/2] fix memory leak in get_task_name() Andrea Claudi 2022-03-02 12:28 ` [PATCH iproute2 v2 1/2] lib/fs: " Andrea Claudi @ 2022-03-02 12:28 ` Andrea Claudi 1 sibling, 0 replies; 12+ messages in thread From: Andrea Claudi @ 2022-03-02 12:28 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 for the fill_res_name_pid() function in the kernel infiniband driver. This commit makes this clear at first glance, using an else branch for the RDMA_NLDEV_ATTR_RES_KERN_NAME case. This also helps coverity to better understand this code and avoid producing a bogus warning complaining about mnl_attr_get_str overwriting comme, 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 | 9 +++++---- 8 files changed, 34 insertions(+), 39 deletions(-) diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c index 3475349d..3abc5b42 100644 --- a/rdma/res-cmid.c +++ b/rdma/res-cmid.c @@ -164,6 +164,10 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx, pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); if (!get_task_name(pid, b)) comm = b; + } 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, @@ -176,12 +180,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 5ed455ea..e691f21f 100644 --- a/rdma/res-cq.c +++ b/rdma/res-cq.c @@ -89,6 +89,10 @@ static int res_cq_line(struct rd *rd, const char *name, int idx, pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); if (!get_task_name(pid, b)) comm = b; + } 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, @@ -106,11 +110,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 fbd52dd5..e300f605 100644 --- a/rdma/res-ctx.c +++ b/rdma/res-ctx.c @@ -23,6 +23,10 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx, pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); if (!get_task_name(pid, b)) comm = b; + } 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, @@ -36,11 +40,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 6a59d9e4..f3a86cac 100644 --- a/rdma/res-mr.c +++ b/rdma/res-mr.c @@ -52,6 +52,10 @@ static int res_mr_line(struct rd *rd, const char *name, int idx, pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); if (!get_task_name(pid, b)) comm = b; + } 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, @@ -70,10 +74,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 a51bb634..7ab3ac6b 100644 --- a/rdma/res-pd.c +++ b/rdma/res-pd.c @@ -39,6 +39,10 @@ static int res_pd_line(struct rd *rd, const char *name, int idx, pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); if (!get_task_name(pid, b)) comm = b; + } 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, @@ -58,11 +62,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 575e0529..d15522e8 100644 --- a/rdma/res-qp.c +++ b/rdma/res-qp.c @@ -151,17 +151,16 @@ static int res_qp_line(struct rd *rd, const char *name, int idx, pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); if (!get_task_name(pid, b)) comm = b; + } 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 945109fc..2944ee4a 100644 --- a/rdma/res-srq.c +++ b/rdma/res-srq.c @@ -179,7 +179,12 @@ static int res_srq_line(struct rd *rd, const char *name, int idx, pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); if (!get_task_name(pid, b)) comm = b; + } 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; @@ -212,11 +217,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 a63b70a4..fe66f4d0 100644 --- a/rdma/stat.c +++ b/rdma/stat.c @@ -253,15 +253,16 @@ static int res_counter_line(struct rd *rd, const char *name, int index, pid = mnl_attr_get_u32(nla_line[RDMA_NLDEV_ATTR_RES_PID]); if (!get_task_name(pid, b)) comm = b; + } 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])) return MNL_CB_OK; - if (nla_line[RDMA_NLDEV_ATTR_RES_KERN_NAME]) - 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.35.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-03-09 13:39 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-03-02 12:28 [PATCH iproute2 v2 0/2] fix memory leak in get_task_name() Andrea Claudi 2022-03-02 12:28 ` [PATCH iproute2 v2 1/2] lib/fs: " Andrea Claudi 2022-03-03 18:56 ` Leon Romanovsky 2022-03-07 18:07 ` Andrea Claudi 2022-03-07 18:12 ` David Ahern 2022-03-09 13:39 ` Leon Romanovsky 2022-03-07 17:58 ` David Ahern 2022-03-07 18:14 ` Stephen Hemminger 2022-03-07 18:21 ` Andrea Claudi 2022-03-07 19:57 ` Stephen Hemminger 2022-03-07 22:38 ` David Ahern 2022-03-02 12:28 ` [PATCH iproute2 v2 2/2] rdma: make RES_PID and RES_KERN_NAME alternative to each other Andrea Claudi
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).