diff mbox series

[iproute2-rc,2/2] rdma: Fix the error of accessing string variable outside the lifecycle

Message ID 20231229065241.554726-3-huangjunxian6@hisilicon.com (mailing list archive)
State Changes Requested
Headers show
Series Bugfixes for rdmatool | expand

Commit Message

Junxian Huang Dec. 29, 2023, 6:52 a.m. UTC 
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(-)

Comments

Junxian Huang Jan. 8, 2024, 1:28 a.m. UTC | #1 
Hi all,

the first patch is replaced by Stephen's latest patches. Are there any
comments to this patch?

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;
Petr Machata Jan. 8, 2024, 11:09 a.m. UTC | #2 
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;
Andrea Claudi Jan. 8, 2024, 4:10 p.m. UTC | #3 
On Mon, Jan 08, 2024 at 09:28:52AM +0800, Junxian Huang wrote:
> 
> Hi all,
> 
> the first patch is replaced by Stephen's latest patches. Are there any
> comments to this patch?
> 
> 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>
> > ---

Hi Junxian,
For future patches, you can have a faster feedback adding to cc the
author of the original patch. In this case it's me, so here's my

Acked-by: Andrea Claudi <aclaudi@redhat.com>
Stephen Hemminger Jan. 8, 2024, 5:25 p.m. UTC | #4 
On Mon, 8 Jan 2024 17:10:04 +0100
Andrea Claudi <aclaudi@redhat.com> wrote:

> On Mon, Jan 08, 2024 at 09:28:52AM +0800, Junxian Huang wrote:
> > 
> > Hi all,
> > 
> > the first patch is replaced by Stephen's latest patches. Are there any
> > comments to this patch?
> > 
> > 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>
> > > ---  
> 
> Hi Junxian,
> For future patches, you can have a faster feedback adding to cc the
> author of the original patch. In this case it's me, so here's my
> 
> Acked-by: Andrea Claudi <aclaudi@redhat.com>
> 

I just merged this one
Junxian Huang Jan. 9, 2024, 1:36 a.m. UTC | #5 
On 2024/1/9 0:10, Andrea Claudi wrote:
> On Mon, Jan 08, 2024 at 09:28:52AM +0800, Junxian Huang wrote:
>>
>> Hi all,
>>
>> the first patch is replaced by Stephen's latest patches. Are there any
>> comments to this patch?
>>
>> 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>
>>> ---
> 
> Hi Junxian,
> For future patches, you can have a faster feedback adding to cc the
> author of the original patch. In this case it's me, so here's my
> 
> Acked-by: Andrea Claudi <aclaudi@redhat.com>
> 

Thanks for the advice!

Junxian
diff mbox series

Patch

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;