Re: [PATCH 1/2] smb: compress: reject Pattern_V1 when not negotiated

Reviewed-by: ChenXiaoSong <[email protected]>
Fixes: a08de24c2b85 ("ksmbd: negotiate and decode SMB2 compression")

See MS-SMB2 3.3.5.2.1.2

By the way, smb_compression_decompress() has too many parameters, 
perhaps we can put them into a structure in the future.

在 2026/8/1 13:19, Anatolii Shumak 写道:
> Pattern_V1 is an optional chained payload type selected during SMB 3.1.1
> compression negotiate. conn->compress_pattern was only consulted when
> building responses, so a peer that negotiated LZ77 with chained support
> could still submit Pattern payloads on the receive path.
> 
> Pass allow_pattern through smb_compression_decompress() and reject
> SMB3_COMPRESS_PATTERN in the chained decoder when it is false.
> 
> Link: https://github.com/namjaejeon/ksmbd/issues/529
> Signed-off-by: Anatolii Shumak <[email protected]>
> ---
>   fs/smb/common/compress/compress.c | 11 ++++++++---
>   fs/smb/common/compress/compress.h |  3 ++-
>   fs/smb/server/compress.c          |  1 +
>   3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/smb/common/compress/compress.c b/fs/smb/common/compress/compress.c
> index b07a317597a4..a4123c8f1c0a 100644
> --- a/fs/smb/common/compress/compress.c
> +++ b/fs/smb/common/compress/compress.c
> @@ -95,6 +95,7 @@ static int smb_decompress_lz77_payload(const u8 **src, u32 *slen, u8 **dst,
>   }
>   
>   static int smb_decompress_chained(__le16 alg, bool allow_chained,
> +				  bool allow_pattern,
>   				  const struct smb2_compression_hdr *hdr,
>   				  u32 slen, void *dst, u32 dlen)
>   {
> @@ -143,6 +144,8 @@ static int smb_decompress_chained(__le16 alg, bool allow_chained,
>   			rc = smb_decompress_none(&src, &remaining, &out,
>   						 &out_remaining, len);
>   		} else if (payload_alg == SMB3_COMPRESS_PATTERN) {
> +			if (!allow_pattern)
> +				return -EINVAL;
>   			rc = smb_decompress_pattern(&src, &remaining, &out,
>   						    &out_remaining, len);
>   		} else if (payload_alg == alg && alg == SMB3_COMPRESS_LZ77) {
> @@ -185,6 +188,7 @@ static int smb_decompress_unchained(__le16 alg,
>    * smb_compression_decompress() - decode an SMB2 compression transform
>    * @alg: negotiated general-purpose compression algorithm
>    * @allow_chained: whether chained transforms were negotiated
> + * @allow_pattern: whether Pattern_V1 payloads were negotiated
>    * @src: transform header followed by compressed payload data
>    * @slen: total number of bytes available at @src
>    * @dst: output buffer for the reconstructed SMB2 message
> @@ -197,7 +201,8 @@ static int smb_decompress_unchained(__le16 alg,
>    * Return: 0 on success, otherwise a negative errno.
>    */
>   int smb_compression_decompress(__le16 alg, bool allow_chained,
> -			       const void *src, u32 slen, void *dst, u32 dlen)
> +			       bool allow_pattern, const void *src, u32 slen,
> +			       void *dst, u32 dlen)
>   {
>   	const struct smb2_compression_hdr *hdr = src;
>   
> @@ -207,8 +212,8 @@ int smb_compression_decompress(__le16 alg, bool allow_chained,
>   		return -EINVAL;
>   
>   	if (hdr->Flags == cpu_to_le16(SMB2_COMPRESSION_FLAG_CHAINED))
> -		return smb_decompress_chained(alg, allow_chained, hdr, slen,
> -					      dst, dlen);
> +		return smb_decompress_chained(alg, allow_chained, allow_pattern,
> +					      hdr, slen, dst, dlen);
>   
>   	if (hdr->Flags != cpu_to_le16(SMB2_COMPRESSION_FLAG_NONE))
>   		return -EINVAL;
> diff --git a/fs/smb/common/compress/compress.h b/fs/smb/common/compress/compress.h
> index 7ace3bf4b664..d6916669f887 100644
> --- a/fs/smb/common/compress/compress.h
> +++ b/fs/smb/common/compress/compress.h
> @@ -20,7 +20,8 @@ static __always_inline bool smb_compress_alg_valid(__le16 alg, bool valid_none)
>   }
>   
>   int smb_compression_decompress(__le16 alg, bool allow_chained,
> -			       const void *src, u32 slen, void *dst, u32 dlen);
> +			       bool allow_pattern, const void *src, u32 slen,
> +			       void *dst, u32 dlen);
>   int smb_compression_compress_chained(__le16 alg, bool allow_pattern,
>   				     const void *src, u32 slen,
>   				     void *dst, u32 *dlen);
> diff --git a/fs/smb/server/compress.c b/fs/smb/server/compress.c
> index 95e48fa6b448..821299888ad3 100644
> --- a/fs/smb/server/compress.c
> +++ b/fs/smb/server/compress.c
> @@ -69,6 +69,7 @@ int ksmbd_decompress_request(struct ksmbd_conn *conn)
>   	*(__be32 *)out = cpu_to_be32(out_size);
>   	rc = smb_compression_decompress(conn->compress_algorithm,
>   					conn->compress_chained,
> +					conn->compress_pattern,
>   					buf, pdu_size, out + 4, out_size);
>   	if (rc) {
>   		kvfree(out);

-- 
ChenXiaoSong <[email protected]>
Chinese Homepage: https://chenxiaosong.com
English Homepage: https://chenxiaosong.com/en