From: Petr Machata <petrm@nvidia.com>
To: Junxian Huang <huangjunxian6@hisilicon.com>
Cc: <jgg@ziepe.ca>, <leon@kernel.org>, <dsahern@gmail.com>,
<stephen@networkplumber.org>,
Chengchang Tang <tangchengchang@huawei.com>,
<netdev@vger.kernel.org>, <linux-rdma@vger.kernel.org>,
<linuxarm@huawei.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH iproute2-rc 2/2] rdma: Fix the error of accessing string variable outside the lifecycle
Date: Mon, 8 Jan 2024 12:09:04 +0100 [thread overview]
Message-ID: <8734v8ozcn.fsf@nvidia.com> (raw)
In-Reply-To: <fb7c85a4-165d-7eda-740a-d11a32cb86c0@hisilicon.com>
Junxian Huang <huangjunxian6@hisilicon.com> writes:
> the first patch is replaced by Stephen's latest patches. Are there any
> comments to this patch?
Yeah, what the code is currently doing is invalid.
Reviewed-by: Petr Machata <petrm@nvidia.com>
> Thanks,
> Junxian
>
> On 2023/12/29 14:52, Junxian Huang wrote:
>> From: wenglianfa <wenglianfa@huawei.com>
>>
>> All these SPRINT_BUF(b) definitions are inside the 'if' block, but
>> accessed outside the 'if' block through the pointers 'comm'. This
>> leads to empty 'comm' attribute when querying resource information.
>> So move the definitions to the beginning of the functions to extend
>> their life cycle.
>>
>> Before:
>> $ rdma res show srq
>> dev hns_0 srqn 0 type BASIC lqpn 18 pdn 5 pid 7775 comm
>>
>> After:
>> $ rdma res show srq
>> dev hns_0 srqn 0 type BASIC lqpn 18 pdn 5 pid 7775 comm ib_send_bw
>>
>> Fixes: 1808f002dfdd ("lib/fs: fix memory leak in get_task_name()")
>> Signed-off-by: wenglianfa <wenglianfa@huawei.com>
>> Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
>> ---
>> rdma/res-cmid.c | 3 +--
>> rdma/res-cq.c | 3 +--
>> rdma/res-ctx.c | 3 +--
>> rdma/res-mr.c | 3 +--
>> rdma/res-pd.c | 3 +--
>> rdma/res-qp.c | 3 +--
>> rdma/res-srq.c | 3 +--
>> rdma/stat.c | 3 +--
>> 8 files changed, 8 insertions(+), 16 deletions(-)
>>
>> diff --git a/rdma/res-cmid.c b/rdma/res-cmid.c
>> index 7371c3a6..595af848 100644
>> --- a/rdma/res-cmid.c
>> +++ b/rdma/res-cmid.c
>> @@ -102,6 +102,7 @@ static int res_cm_id_line(struct rd *rd, const char *name, int idx,
>> uint32_t lqpn = 0, ps;
>> uint32_t cm_idn = 0;
>> char *comm = NULL;
>> + SPRINT_BUF(b);
>>
>> if (!nla_line[RDMA_NLDEV_ATTR_RES_STATE] ||
>> !nla_line[RDMA_NLDEV_ATTR_RES_PS])
>> @@ -159,8 +160,6 @@ 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]);
>> if (!get_task_name(pid, b, sizeof(b)))
>> comm = b;
>> diff --git a/rdma/res-cq.c b/rdma/res-cq.c
>> index 2cfa4994..80994a03 100644
>> --- a/rdma/res-cq.c
>> +++ b/rdma/res-cq.c
>> @@ -63,6 +63,7 @@ static int res_cq_line(struct rd *rd, const char *name, int idx,
>> uint32_t cqn = 0;
>> uint64_t users;
>> uint32_t cqe;
>> + SPRINT_BUF(b);
>>
>> if (!nla_line[RDMA_NLDEV_ATTR_RES_CQE] ||
>> !nla_line[RDMA_NLDEV_ATTR_RES_USECNT])
>> @@ -84,8 +85,6 @@ 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]);
>> if (!get_task_name(pid, b, sizeof(b)))
>> comm = b;
>> diff --git a/rdma/res-ctx.c b/rdma/res-ctx.c
>> index 500186d9..99736ea0 100644
>> --- a/rdma/res-ctx.c
>> +++ b/rdma/res-ctx.c
>> @@ -13,13 +13,12 @@ static int res_ctx_line(struct rd *rd, const char *name, int idx,
>> char *comm = NULL;
>> uint32_t ctxn = 0;
>> uint32_t pid = 0;
>> + SPRINT_BUF(b);
>>
>> if (!nla_line[RDMA_NLDEV_ATTR_RES_CTXN])
>> 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]);
>> if (!get_task_name(pid, b, sizeof(b)))
>> comm = b;
>> diff --git a/rdma/res-mr.c b/rdma/res-mr.c
>> index fb48d5df..e6c81d11 100644
>> --- a/rdma/res-mr.c
>> +++ b/rdma/res-mr.c
>> @@ -30,6 +30,7 @@ static int res_mr_line(struct rd *rd, const char *name, int idx,
>> uint32_t pdn = 0;
>> uint32_t mrn = 0;
>> uint32_t pid = 0;
>> + SPRINT_BUF(b);
>>
>> if (!nla_line[RDMA_NLDEV_ATTR_RES_MRLEN])
>> return MNL_CB_ERROR;
>> @@ -47,8 +48,6 @@ 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]);
>> if (!get_task_name(pid, b, sizeof(b)))
>> comm = b;
>> diff --git a/rdma/res-pd.c b/rdma/res-pd.c
>> index 66f91f42..0dbb310a 100644
>> --- a/rdma/res-pd.c
>> +++ b/rdma/res-pd.c
>> @@ -16,6 +16,7 @@ static int res_pd_line(struct rd *rd, const char *name, int idx,
>> uint32_t pid = 0;
>> uint32_t pdn = 0;
>> uint64_t users;
>> + SPRINT_BUF(b);
>>
>> if (!nla_line[RDMA_NLDEV_ATTR_RES_USECNT])
>> return MNL_CB_ERROR;
>> @@ -34,8 +35,6 @@ 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]);
>> if (!get_task_name(pid, b, sizeof(b)))
>> comm = b;
>> diff --git a/rdma/res-qp.c b/rdma/res-qp.c
>> index c180a97e..cd2f4aa2 100644
>> --- a/rdma/res-qp.c
>> +++ b/rdma/res-qp.c
>> @@ -86,6 +86,7 @@ static int res_qp_line(struct rd *rd, const char *name, int idx,
>> uint32_t port = 0, pid = 0;
>> uint32_t pdn = 0;
>> char *comm = NULL;
>> + SPRINT_BUF(b);
>>
>> if (!nla_line[RDMA_NLDEV_ATTR_RES_LQPN] ||
>> !nla_line[RDMA_NLDEV_ATTR_RES_SQ_PSN] ||
>> @@ -146,8 +147,6 @@ 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]);
>> if (!get_task_name(pid, b, sizeof(b)))
>> comm = b;
>> diff --git a/rdma/res-srq.c b/rdma/res-srq.c
>> index cf9209d7..758bb193 100644
>> --- a/rdma/res-srq.c
>> +++ b/rdma/res-srq.c
>> @@ -183,13 +183,12 @@ static int res_srq_line(struct rd *rd, const char *name, int idx,
>> char qp_str[MAX_QP_STR_LEN] = {};
>> char *comm = NULL;
>> uint8_t type = 0;
>> + SPRINT_BUF(b);
>>
>> if (!nla_line[RDMA_NLDEV_ATTR_RES_SRQN])
>> 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]);
>> if (!get_task_name(pid, b, sizeof(b)))
>> comm = b;
>> diff --git a/rdma/stat.c b/rdma/stat.c
>> index 3df2c98f..c7dde680 100644
>> --- a/rdma/stat.c
>> +++ b/rdma/stat.c
>> @@ -223,6 +223,7 @@ static int res_counter_line(struct rd *rd, const char *name, int index,
>> struct nlattr *hwc_table, *qp_table;
>> struct nlattr *nla_entry;
>> const char *comm = NULL;
>> + SPRINT_BUF(b);
>> bool isfirst;
>> int err;
>>
>> @@ -248,8 +249,6 @@ 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]);
>> if (!get_task_name(pid, b, sizeof(b)))
>> comm = b;
next prev parent reply other threads:[~2024-01-08 11:10 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-29 6:52 [PATCH iproute2-rc 0/2] Bugfixes for rdmatool Junxian Huang
2023-12-29 6:52 ` [PATCH iproute2-rc 1/2] rdma: Fix core dump when pretty is used Junxian Huang
2023-12-29 17:21 ` Stephen Hemminger
2024-01-02 7:44 ` Chengchang Tang
2024-01-02 8:32 ` Leon Romanovsky
2024-01-02 12:06 ` Chengchang Tang
2024-01-02 12:21 ` Leon Romanovsky
2024-01-02 16:27 ` Stephen Hemminger
2024-01-02 19:17 ` Leon Romanovsky
2024-01-02 19:29 ` David Ahern
2023-12-29 6:52 ` [PATCH iproute2-rc 2/2] rdma: Fix the error of accessing string variable outside the lifecycle Junxian Huang
2024-01-08 1:28 ` Junxian Huang
2024-01-08 11:09 ` Petr Machata [this message]
2024-01-08 16:10 ` Andrea Claudi
2024-01-08 17:25 ` Stephen Hemminger
2024-01-09 1:36 ` Junxian Huang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8734v8ozcn.fsf@nvidia.com \
--to=petrm@nvidia.com \
--cc=dsahern@gmail.com \
--cc=huangjunxian6@hisilicon.com \
--cc=jgg@ziepe.ca \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=netdev@vger.kernel.org \
--cc=stephen@networkplumber.org \
--cc=tangchengchang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).