RE: [PATCH net-next v3] tipc: replace deprecated strcpy with strscpy in tipc_bearer_get_name()

[Tung Quang Nguyen]

>Subject: Re: [PATCH net-next v3] tipc: replace deprecated strcpy with strscpy in
>tipc_bearer_get_name()
>
>> net tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
>> net-next tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>
>Thank you. I tried the correct net-next tree. The same patch applied
>successfully here also as shown below.
>
><path of the linux tree>/net-next$ ls
>arch   certs    CREDITS  Documentation  fs       init      ipc     Kconfig  lib
>MAINTAINERS  mm   README  samples  security  tools  virt
>block  COPYING  crypto   drivers        include  io_uring  Kbuild  kernel   LICENSES
>Makefile     net  rust    scripts  sound     usr
><path of the linux tree>/net-next$ git remote -v
>origin	https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>(fetch)
>origin	https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>(push)
><path of the linux tree>/net-next$ git am ../../patches/v3-0001-tipc-replace-
>deprecated-strcpy-with-strscpy-in-ti.patch
>Applying: tipc: replace deprecated strcpy with strscpy in
>tipc_bearer_get_name() <path of the linux tree>/net-next$ git branch
>* main
><path of the linux tree>/net-next$
>
>Am I missing something?

I see that your patch was applied again successfully after failure.
There is one feedback from AI review that I see valid:
[...]
> @@ -209,7 +209,7 @@ int tipc_bearer_get_name(struct net *net, char *name, u32 bearer_id)
>  	if (!b)
>  		return -EINVAL;
>  
> -	strcpy(name, b->name);
> +	strscpy(name, b->name, TIPC_MAX_BEARER_NAME);
This isn't a bug, but the destination capacity is now hard-coded in the
body while the interface says nothing about it.  The declaration in
net/tipc/bearer.h is still:
	int tipc_bearer_get_name(struct net *net, char *name, u32 bearer_id);
and the comment directly above the implementation only says:
	 *     @name: a pointer to the buffer where the name will be stored.
Should the TIPC_MAX_BEARER_NAME requirement be spelled out in the comment
or in the prototype, so that the interface and the body agree?
In the current tree there is no behavioural change: the only caller is
__tipc_nl_add_monitor() in net/tipc/monitor.c, which declares
char bearer_name[TIPC_MAX_BEARER_NAME], and the source is
char name[TIPC_MAX_BEARER_NAME] in struct tipc_bearer, so the copy can
neither truncate nor overflow.
The strscpy() return value is also discarded, so a future caller passing a
smaller buffer would get a silently truncated name plus a return of 0.
Would passing a size_t len from the caller (sizeof(bearer_name)) and
forwarding strscpy()'s -E2BIG through the existing int return be preferable
to a hard-coded bound?
[...]

Please update your patch and send the new version after Aug 31st. (Because net-next is closed now)