* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.