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

[Benjamin Coddington <bcodding@redhat.com>]

On 17 Dec 2024, at 12:07, Trond Myklebust wrote:

> On Tue, 2024-12-17 at 16:51 +0000, Trond Myklebust wrote:
>> 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.
>
> Just to clarify: I'm not strongly against the patch itself. However it
> would seem premature to consider this a bug, let alone a stable fix
> candidate. 
>
> Has anyone ever seen a buffer overflow here? If so, under which
> circumstances?

No, there's no way in the current kernel, but I suppose both
nfs_sysfs_link_rpc_client() as well as rpc_create() are exported, so we
could end up having some other part of the kernel send a long "uniq" or
create a client with a long cl_program->name.  That change might escape our
review.

Probably not a bad idea to send it back to stable IMO, since a change like
that could get back there too.

Reviewed-by: Benjamin Coddington <bcodding@redhat.com>

Ben