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

[Junxian Huang <huangjunxian6@hisilicon.com>]

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;