Re: [PATCH 2/3] smb: client: check posix_info_parse() in cifs_posix_to_fattr()

Hi Ren and Zihan,

Thank you for the patch.

I have a concern regarding the placement of the error check. It seems
like the check for posix_info_parse() should occur later in the
function, specifically before the two lines that utilize the parsed
output:

sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER);
sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP);

Perhaps we should use something like:

if (!posix_info_parse(info, NULL, &parsed)) {
      sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER);
      sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP);
}

Could you please take a look and let me know your thoughts?

Thanks,
Steve French

On Wed, Jul 1, 2026 at 5:26 AM Ren Wei <[email protected]> wrote:
>
> From: Zihan Xi <[email protected]>
>
> posix_info_parse() returns an error without filling the output structure.
> Check the return value before calling sid_to_id() to avoid using
> uninitialized owner/group SID data.
>
> Fixes: 9934430e2178 ("SMB3.1.1: Fix ids returned in POSIX query dir")
> Cc: [email protected]
> Signed-off-by: Zihan Xi <[email protected]>
> Signed-off-by: Ren Wei <[email protected]>
> ---
>  fs/smb/client/readdir.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> index 1ff77f3d1de0..df46aa919c27 100644
> --- a/fs/smb/client/readdir.c
> +++ b/fs/smb/client/readdir.c
> @@ -244,9 +244,11 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info,
>  {
>         struct smb2_posix_info_parsed parsed;
>
> -       posix_info_parse(info, NULL, &parsed);
> -
>         memset(fattr, 0, sizeof(*fattr));
> +       if (posix_info_parse(info, NULL, &parsed) < 0) {
> +               cifs_dbg(VFS, "Invalid POSIX info payload\n");
> +               return;
> +       }
>         fattr->cf_uniqueid = le64_to_cpu(info->Inode);
>         fattr->cf_bytes = le64_to_cpu(info->AllocationSize);
>         fattr->cf_eof = le64_to_cpu(info->EndOfFile);
> --
> 2.43.0
>
>


-- 
Thanks,

Steve