Re: [PATCH] smb: client: tighten validate_t2() offset bounds against actual buffer size

[Paulo Alcantara <pc@manguebit.org>]

Frank Sorenson <sorenson@redhat.com> writes:

> validate_t2() rejects ParameterOffset and DataOffset only when they
> exceed 1024, far below the allocated buffer ceiling.  Raise the
> individual offset guard to CIFSMaxBufSize + MAX_CIFS_HDR_SIZE.  Add
> joint offset+count checks against frame_end — bytes from hdr.Protocol
> to the end of the received payload, derived from WordCount and BCC to
> match smbCalcSize() — so a large offset with zero count cannot reach
> beyond the received frame.
>
> In CIFSFindFirst and CIFSFindNext, bound lnoff <= DataCount: since
> validate_t2() guarantees data_off + DataCount <= frame_end, this
> transitively bounds data_off + lnoff within the frame.
>
> Add min_param_size and min_data_size parameters to verify the offset
> clears the byte past ByteCount and that offset + struct size fits in the
> frame.  validate_t2() computes the minimum valid offset dynamically as
> frame_end - BCC, which accounts for SetupCount > 0 (WordCount > 10)
> responses where extra setup words shift ByteCount and the data area
> later.  Non-obvious sizes at the call sites:
>
> - CIFSPOSIXCreate: sizeof(OPEN_PSX_RSP) + sizeof(FILE_UNIX_BASIC_INFO),
>   as the caller memcpys FILE_UNIX_BASIC_INFO immediately after OPEN_PSX_RSP.
> - CIFSSMBQPathInfo: legacy ? offsetof(FILE_INFO_STANDARD, EASize) :
>   sizeof(FILE_ALL_INFO); the legacy path skips EASize intentionally.
> - CIFSSMBQAllEAs: offsetof(struct fealist, list), not sizeof, because an
>   empty EA list returns DataCount=4 (list_len only).
> - CIFSSMBPosixLock: (0, sizeof(struct cifs_posix_lock)) — pSMBr aliases
>   the small request buffer on the waitFlag path; defer
>   cifs_small_buf_release(pSMB) to plk_err_exit so the pLockData branch
>   does not read from freed memory.  validate_t2()'s frame_end bound covers
>   both the waitFlag (small buffer) and !waitFlag (transport-chosen buffer)
>   paths correctly.
> - CIFSGetDFSRefer: (0, 0) — parse_dfs_referrals() reads from the fixed
>   dfs_data struct offset, not from DataOffset; add an explicit check that
>   dfs_data + DataCount fits within the received frame.

Besides all these LLM-generated messages and comments, would you have a
reproducer or a real use case that would require such changes?