Re: [PATCH] NFS: Fix potential buffer overflowin nfs_sysfs_link_rpc_client()

[Trond Myklebust <trondmy@hammerspace.com>]

On Wed, 2024-12-18 at 00:13 +0800, Gax-c wrote:
> From: Zichen Xie <zichenxie0106@gmail.com>
> 
> name is char[64] where the size of clnt->cl_program->name remains
> unknown. Invoking strcat() directly will also lead to potential
> buffer
> overflow. Change them to strscpy() and strncat() to fix potential
> issues.

What makes you think that clnt->cl_program->name is unknown?

All calls to nfs_sysfs_link_rpc_client() use well known RPC clients for
which the cl_program is pointing to one of nlm_program, nfs_program or
nfsacl_program. So we know very well the sizes of clnt->cl_program-
>name.

> 
> Fixes: e13b549319a6 ("NFS: Add sysfs links to sunrpc clients for
> nfs_clients")
> Signed-off-by: Zichen Xie <zichenxie0106@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/nfs/sysfs.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
> index bf378ecd5d9f..7b59a40d40c0 100644
> --- a/fs/nfs/sysfs.c
> +++ b/fs/nfs/sysfs.c
> @@ -280,9 +280,9 @@ void nfs_sysfs_link_rpc_client(struct nfs_server
> *server,
>  	char name[RPC_CLIENT_NAME_SIZE];
>  	int ret;
>  
> -	strcpy(name, clnt->cl_program->name);
> -	strcat(name, uniq ? uniq : "");
> -	strcat(name, "_client");
> +	strscpy(name, clnt->cl_program->name, sizeof(name));
> +	strncat(name, uniq ? uniq : "", sizeof(name) - strlen(name)
> - 1);
> +	strncat(name, "_client", sizeof(name) - strlen(name) - 1);
>  
>  	ret = sysfs_create_link_nowarn(&server->kobj,
>  						&clnt->cl_sysfs-
> >kobject, name);

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com