linux-kernel - Re: [PATCH v2] smb: client: correctly handle ErrorContextData as a flexible array

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH2r5muQG67z3MEh2gp_Ww=yv1AAZxHORydR9x0ABCeufUjfWQ@mail.gmail.com>
Date: Sat, 18 Jan 2025 13:33:07 -0600
From: Steve French <smfrench@...il.com>
To: Liang Jie <buaajxlj@....com>
Cc: sfrench@...ba.org, tom@...pey.com, pc@...guebit.com, 
	ronniesahlberg@...il.com, sprasad@...rosoft.com, fanggeng@...iang.com, 
	yangchen11@...iang.com, bharathsm@...rosoft.com, linux-cifs@...r.kernel.org, 
	samba-technical@...ts.samba.org, linux-kernel@...r.kernel.org, 
	Liang Jie <liangjie@...iang.com>
Subject: Re: [PATCH v2] smb: client: correctly handle ErrorContextData as a
 flexible array

updated patch in for-next now


On Sat, Jan 18, 2025 at 6:37 AM Liang Jie <buaajxlj@....com> wrote:
>
> From: Liang Jie <liangjie@...iang.com>
>
> The `smb2_symlink_err_rsp` structure was previously defined with
> `ErrorContextData` as a single `__u8` byte. However, the `ErrorContextData`
> field is intended to be a variable-length array based on `ErrorDataLength`.
> This mismatch leads to incorrect pointer arithmetic and potential memory
> access issues when processing error contexts.
>
> Updates the `ErrorContextData` field to be a flexible array
> (`__u8 ErrorContextData[]`). Additionally, it modifies the corresponding
> casts in the `symlink_data()` function to properly handle the flexible
> array, ensuring correct memory calculations and data handling.
>
> These changes improve the robustness of SMB2 symlink error processing.
>
> Signed-off-by: Liang Jie <liangjie@...iang.com>
> Suggested-by: Tom Talpey <tom@...pey.com>
> ---
>
> Changes in v2:
> - Add the __counted_by_le attribute to reference the ErrorDataLength protocol field.
> - Link to v1: https://lore.kernel.org/all/20250116072948.682402-1-buaajxlj@163.com/
>
>  fs/smb/client/smb2file.c | 4 ++--
>  fs/smb/client/smb2pdu.h  | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/smb/client/smb2file.c b/fs/smb/client/smb2file.c
> index e836bc2193dd..9ec44eab8dbc 100644
> --- a/fs/smb/client/smb2file.c
> +++ b/fs/smb/client/smb2file.c
> @@ -42,14 +42,14 @@ static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov)
>                 end = (struct smb2_error_context_rsp *)((u8 *)err + iov->iov_len);
>                 do {
>                         if (le32_to_cpu(p->ErrorId) == SMB2_ERROR_ID_DEFAULT) {
> -                               sym = (struct smb2_symlink_err_rsp *)&p->ErrorContextData;
> +                               sym = (struct smb2_symlink_err_rsp *)p->ErrorContextData;
>                                 break;
>                         }
>                         cifs_dbg(FYI, "%s: skipping unhandled error context: 0x%x\n",
>                                  __func__, le32_to_cpu(p->ErrorId));
>
>                         len = ALIGN(le32_to_cpu(p->ErrorDataLength), 8);
> -                       p = (struct smb2_error_context_rsp *)((u8 *)&p->ErrorContextData + len);
> +                       p = (struct smb2_error_context_rsp *)(p->ErrorContextData + len);
>                 } while (p < end);
>         } else if (le32_to_cpu(err->ByteCount) >= sizeof(*sym) &&
>                    iov->iov_len >= SMB2_SYMLINK_STRUCT_SIZE) {
> diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h
> index 076d9e83e1a0..3c09a58dfd07 100644
> --- a/fs/smb/client/smb2pdu.h
> +++ b/fs/smb/client/smb2pdu.h
> @@ -79,7 +79,7 @@ struct smb2_symlink_err_rsp {
>  struct smb2_error_context_rsp {
>         __le32 ErrorDataLength;
>         __le32 ErrorId;
> -       __u8  ErrorContextData; /* ErrorDataLength long array */
> +       __u8  ErrorContextData[] __counted_by_le(ErrorDataLength);
>  } __packed;
>
>  /* ErrorId values */
> --
> 2.25.1
>
>


--
Thanks,

Steve