Re: [bug report] smb: client: batch SRV_COPYCHUNK entries to cut roundtrips

[Henrique Carvalho <henrique.carvalho@suse.com>]

Hi Dan,

Thanks for the report.

On 9/18/25 6:44 AM, Dan Carpenter wrote:
> Hello Henrique Carvalho,
>
> Commit 4e3acdf94c89 ("smb: client: batch SRV_COPYCHUNK entries to cut
> roundtrips") from Sep 16, 2025 (linux-next), leads to the following
> Smatch static checker warning:
>
> 	fs/smb/client/smb2ops.c:1937 smb2_copychunk_range()
> 	error: uninitialized symbol 'ret_data_len'.
>
> --> 1937                 if (ret_data_len != sizeof(struct copychunk_ioctl_rsp)) {
>                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Using unintialized variables doesn't work because runtime checkers like
> UBSan will complain as well as static checkers.

I will update the patch to initialize ret_data_len.

> But the other issue is that this code treats -EINVAL as having a special
> meaning.  -EINVAL is the most common error code.  All the places which
> treat -EINVAL as having a special meaning end up buggy in the end.  Mostly
> they start out buggy, but even the ones which aren't buggy from the start
> end up buggy over time.

-EINVAL is mapped from the SMB2 server returned error
STATUS_INVALID_PARAMETER, so I don't know if there is anything that can
be done here.

After your email, I considered returning -EOPNOTSUPP instead to trigger
kernel-side copy fallback, but returning -EINVAL also seems to have the
practical advantage of having a successful copy, but with fewer network
roundtrips (apparently due to userspace fallback, but I haven't digged
too much into it).

If there's consensus that changing this error mapping would improve the
overall robustness of the code path, I'm open to that approach.

-- 
Henrique
SUSE Labs