[PATCH] net/smc: replace strncpy with strscpy for ib_name

Kriish Sharma posted 1 patch 5 months ago
net/smc/smc_pnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] net/smc: replace strncpy with strscpy for ib_name
Posted by Kriish Sharma 5 months ago
Replace the deprecated strncpy() with strscpy() for ib_name in
smc_pnet_add_ib(). The destination buffer should be NUL-terminated and
does not require any trailing NUL-padding. Since ib_name is a fixed-size
array, the two-argument form of strscpy() is sufficient and preferred.

Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>
---
 net/smc/smc_pnet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index 76ad29e31d60..b90337f86e83 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -450,7 +450,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
 		return -ENOMEM;
 	new_pe->type = SMC_PNET_IB;
 	memcpy(new_pe->pnet_name, pnet_name, SMC_MAX_PNETID_LEN);
-	strncpy(new_pe->ib_name, ib_name, IB_DEVICE_NAME_MAX);
+	strscpy(new_pe->ib_name, ib_name);
 	new_pe->ib_port = ib_port;
 
 	new_ibdev = true;
-- 
2.34.1
Re: [PATCH] net/smc: replace strncpy with strscpy for ib_name
Posted by Leon Romanovsky 5 months ago
On Mon, Sep 08, 2025 at 06:09:13PM +0000, Kriish Sharma wrote:
> Replace the deprecated strncpy() with strscpy() for ib_name in
> smc_pnet_add_ib(). The destination buffer should be NUL-terminated and
> does not require any trailing NUL-padding. Since ib_name is a fixed-size
> array, the two-argument form of strscpy() is sufficient and preferred.
> 
> Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>
> ---
>  net/smc/smc_pnet.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
> index 76ad29e31d60..b90337f86e83 100644
> --- a/net/smc/smc_pnet.c
> +++ b/net/smc/smc_pnet.c
> @@ -450,7 +450,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
>  		return -ENOMEM;
>  	new_pe->type = SMC_PNET_IB;
>  	memcpy(new_pe->pnet_name, pnet_name, SMC_MAX_PNETID_LEN);
> -	strncpy(new_pe->ib_name, ib_name, IB_DEVICE_NAME_MAX);
> +	strscpy(new_pe->ib_name, ib_name);

It is worth to mention that caching ib_name is wrong as IB/core provides
IB device rename functionality.

Thanks

>  	new_pe->ib_port = ib_port;
>  
>  	new_ibdev = true;
> -- 
> 2.34.1
> 
>
Re: [PATCH] net/smc: replace strncpy with strscpy for ib_name
Posted by Mahanta Jambigi 4 months, 4 weeks ago
On 10/09/25 3:31 pm, Leon Romanovsky wrote:
>> --- a/net/smc/smc_pnet.c
>> +++ b/net/smc/smc_pnet.c
>> @@ -450,7 +450,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
>>  		return -ENOMEM;
>>  	new_pe->type = SMC_PNET_IB;
>>  	memcpy(new_pe->pnet_name, pnet_name, SMC_MAX_PNETID_LEN);
>> -	strncpy(new_pe->ib_name, ib_name, IB_DEVICE_NAME_MAX);
>> +	strscpy(new_pe->ib_name, ib_name);
> 
> It is worth to mention that caching ib_name is wrong as IB/core provides
> IB device rename functionality.

In our case we hit this code path where we pass *PCI_ID*
as the *ib_name* using *smc_pnet* tool(smc_pnet -a <pnet_name> -D
<PCI_ID>). I believe PCI_ID will not change, so caching it here is fine.
Re: [PATCH] net/smc: replace strncpy with strscpy for ib_name
Posted by Leon Romanovsky 4 months, 4 weeks ago
On Fri, Sep 12, 2025 at 01:18:52PM +0530, Mahanta Jambigi wrote:
> On 10/09/25 3:31 pm, Leon Romanovsky wrote:
> >> --- a/net/smc/smc_pnet.c
> >> +++ b/net/smc/smc_pnet.c
> >> @@ -450,7 +450,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
> >>  		return -ENOMEM;
> >>  	new_pe->type = SMC_PNET_IB;
> >>  	memcpy(new_pe->pnet_name, pnet_name, SMC_MAX_PNETID_LEN);
> >> -	strncpy(new_pe->ib_name, ib_name, IB_DEVICE_NAME_MAX);
> >> +	strscpy(new_pe->ib_name, ib_name);
> > 
> > It is worth to mention that caching ib_name is wrong as IB/core provides
> > IB device rename functionality.
> 
> In our case we hit this code path where we pass *PCI_ID*
> as the *ib_name* using *smc_pnet* tool(smc_pnet -a <pnet_name> -D
> <PCI_ID>). I believe PCI_ID will not change, so caching it here is fine.

If I remember, you are reporting that cached ib_name through netlink much later.

The caching itself is not an issue, but incorrect reported name can be seen as
a wrong thing to do.

Thanks
Re: [PATCH] net/smc: replace strncpy with strscpy for ib_name
Posted by Mahanta Jambigi 4 months, 3 weeks ago
On 12/09/25 2:37 pm, Leon Romanovsky wrote:
> On Fri, Sep 12, 2025 at 01:18:52PM +0530, Mahanta Jambigi wrote:
>> On 10/09/25 3:31 pm, Leon Romanovsky wrote:
>>>> --- a/net/smc/smc_pnet.c
>>>> +++ b/net/smc/smc_pnet.c
>>>> @@ -450,7 +450,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
>>>>  		return -ENOMEM;
>>>>  	new_pe->type = SMC_PNET_IB;
>>>>  	memcpy(new_pe->pnet_name, pnet_name, SMC_MAX_PNETID_LEN);
>>>> -	strncpy(new_pe->ib_name, ib_name, IB_DEVICE_NAME_MAX);
>>>> +	strscpy(new_pe->ib_name, ib_name);
>>>
>>> It is worth to mention that caching ib_name is wrong as IB/core provides
>>> IB device rename functionality.
>>
>> In our case we hit this code path where we pass *PCI_ID*
>> as the *ib_name* using *smc_pnet* tool(smc_pnet -a <pnet_name> -D
>> <PCI_ID>). I believe PCI_ID will not change, so caching it here is fine.
> 
> If I remember, you are reporting that cached ib_name through netlink much later.
> 
> The caching itself is not an issue, but incorrect reported name can be seen as
> a wrong thing to do.

In what case we can see this incorrect reported name, could you please
elaborate.
Re: [PATCH] net/smc: replace strncpy with strscpy for ib_name
Posted by Leon Romanovsky 4 months, 3 weeks ago
On Mon, Sep 15, 2025 at 12:24:16PM +0530, Mahanta Jambigi wrote:
> On 12/09/25 2:37 pm, Leon Romanovsky wrote:
> > On Fri, Sep 12, 2025 at 01:18:52PM +0530, Mahanta Jambigi wrote:
> >> On 10/09/25 3:31 pm, Leon Romanovsky wrote:
> >>>> --- a/net/smc/smc_pnet.c
> >>>> +++ b/net/smc/smc_pnet.c
> >>>> @@ -450,7 +450,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
> >>>>  		return -ENOMEM;
> >>>>  	new_pe->type = SMC_PNET_IB;
> >>>>  	memcpy(new_pe->pnet_name, pnet_name, SMC_MAX_PNETID_LEN);
> >>>> -	strncpy(new_pe->ib_name, ib_name, IB_DEVICE_NAME_MAX);
> >>>> +	strscpy(new_pe->ib_name, ib_name);
> >>>
> >>> It is worth to mention that caching ib_name is wrong as IB/core provides
> >>> IB device rename functionality.
> >>
> >> In our case we hit this code path where we pass *PCI_ID*
> >> as the *ib_name* using *smc_pnet* tool(smc_pnet -a <pnet_name> -D
> >> <PCI_ID>). I believe PCI_ID will not change, so caching it here is fine.
> > 
> > If I remember, you are reporting that cached ib_name through netlink much later.
> > 
> > The caching itself is not an issue, but incorrect reported name can be seen as
> > a wrong thing to do.
> 
> In what case we can see this incorrect reported name, could you please
> elaborate.

Did you open net/smc/smc_pnet.c?

Everything that uses ib_name in that file is incorrect.

From glance look:
1. smc_pnet_find_ib() returns completely random results if device is
renamed in parallel.
2. SMC_PNETID_GET returns wrong names. It returns cached name which
doesn't exist anymore.

IB devices have stable indexes in similar way to netdevice. The code
should rely on it and not on the name.

Thanks
Re: [PATCH] net/smc: replace strncpy with strscpy for ib_name
Posted by Mahanta Jambigi 4 months, 3 weeks ago

On 15/09/25 1:32 pm, Leon Romanovsky wrote:
> On Mon, Sep 15, 2025 at 12:24:16PM +0530, Mahanta Jambigi wrote:
>> On 12/09/25 2:37 pm, Leon Romanovsky wrote:
>>> On Fri, Sep 12, 2025 at 01:18:52PM +0530, Mahanta Jambigi wrote:
>>>> On 10/09/25 3:31 pm, Leon Romanovsky wrote:
>>>>>> --- a/net/smc/smc_pnet.c
>>>>>> +++ b/net/smc/smc_pnet.c
>>>>>> @@ -450,7 +450,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
>>>>>>  		return -ENOMEM;
>>>>>>  	new_pe->type = SMC_PNET_IB;
>>>>>>  	memcpy(new_pe->pnet_name, pnet_name, SMC_MAX_PNETID_LEN);
>>>>>> -	strncpy(new_pe->ib_name, ib_name, IB_DEVICE_NAME_MAX);
>>>>>> +	strscpy(new_pe->ib_name, ib_name);
>>>>>
>>>>> It is worth to mention that caching ib_name is wrong as IB/core provides
>>>>> IB device rename functionality.
>>>>
>>>> In our case we hit this code path where we pass *PCI_ID*
>>>> as the *ib_name* using *smc_pnet* tool(smc_pnet -a <pnet_name> -D
>>>> <PCI_ID>). I believe PCI_ID will not change, so caching it here is fine.
>>>
>>> If I remember, you are reporting that cached ib_name through netlink much later.
>>>
>>> The caching itself is not an issue, but incorrect reported name can be seen as
>>> a wrong thing to do.
>>
>> In what case we can see this incorrect reported name, could you please
>> elaborate.
> 
> Did you open net/smc/smc_pnet.c?

Yes, I have gone through it focusing on *ib_name* usage.

> 
> Everything that uses ib_name in that file is incorrect.
> 
> From glance look:
> 1. smc_pnet_find_ib() returns completely random results if device is
> renamed in parallel.

I agree. Currently we are *not* handling the change of IB device names &
when that happens the cached *ib_name* doesn't match anymore with the
changed ib_name. We are looking into this internally.
Re: [PATCH] net/smc: replace strncpy with strscpy for ib_name
Posted by Dust Li 5 months ago
On 2025-09-08 18:09:13, Kriish Sharma wrote:
>Replace the deprecated strncpy() with strscpy() for ib_name in
>smc_pnet_add_ib(). The destination buffer should be NUL-terminated and
>does not require any trailing NUL-padding. Since ib_name is a fixed-size
>array, the two-argument form of strscpy() is sufficient and preferred.
>
>Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>

I think d250f14f5f0754ce2d05d9c0ce778e4a51f488b0 has already done the
same thing.

Best regards,
Dust


>---
> net/smc/smc_pnet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
>index 76ad29e31d60..b90337f86e83 100644
>--- a/net/smc/smc_pnet.c
>+++ b/net/smc/smc_pnet.c
>@@ -450,7 +450,7 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
> 		return -ENOMEM;
> 	new_pe->type = SMC_PNET_IB;
> 	memcpy(new_pe->pnet_name, pnet_name, SMC_MAX_PNETID_LEN);
>-	strncpy(new_pe->ib_name, ib_name, IB_DEVICE_NAME_MAX);
>+	strscpy(new_pe->ib_name, ib_name);
> 	new_pe->ib_port = ib_port;
> 
> 	new_ibdev = true;
>-- 
>2.34.1