Re: [PATCH] smb/server: fix SMB2_MIN_SUPPORTED_HEADER_SIZE value

[ChenXiaoSong <chenxiaosong.chenxiaosong@linux.dev>]

OK, I will send v2 soon.
Thanks,
ChenXiaoSong.

On December 19, 2025 6:32:18 PM GMT+08:00, Namjae Jeon <linkinjeon@kernel.org> wrote:
>On Fri, Dec 19, 2025 at 5:18 PM David Howells <dhowells@redhat.com> wrote:
>>
>> Namjae Jeon <linkinjeon@kernel.org> wrote:
>>
>> > > We should rename them to `SMB1_MIN_SUPPORTED_PDU_SIZE` and
>> > > `SMB2_MIN_SUPPORTED_PDU_SIZE`.
>> > >
>> > > But we "should not" add "+4" to them.
>> > Not adding the +4 will trigger a slab-out-of-bounds issue.
>> > You should check ksmbd_smb2_check_message() and
>> > ksmbd_negotiate_smb_dialect() as well as ksmbd_conn_handler_loop().
>> > ...
>> > >    pdu_size = get_rfc1002_len(hdr_buf);
>> > >    ...
>> > >    if (pdu_size < SMB1_MIN_SUPPORTED_HEADER_SIZE)
>> > >    ...
>> > >    if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
>>
>> As previously mentioned, the problem I have with the +4 accounting for the
>> RFC1002 header is that the size value in pdu_size does not include the size of
>> the RFC1002 header, so the comparison seems to be allowing an overlong header.
>>
>> However, I think the +4 actually makes sense for SMB2/3 - assuming the +4
>> isn't actually for the RFC1002 size, but is rather for the StructureSize of
>> the operation header that follows immediately after the smb2_hdr.
>Right.
>>
>> If that's the case, I would guess that the SMB1 variant might want a +2 to
>> allow for the BCC field...  but according to the code in cifs side that I've
>> looked at, some servers may only provide half a BCC field - or maybe even
>> forget to put it in entirely.
>ksmbd only handles SMB1 negotiate requests in smb1 protocol, And the
>BCC (Byte Count) field of smb1 negotiate request must be greater than
>or equal to 2. This is why I originally treated any SMB1 request
>smaller than smb_hdr + 4 as an invalid packet. However, even if we add
>+2, it will be no problem because ksmbd_negotiate_smb_dialect() checks
>to verify if the BCC field is less than 2.
>
>ChenXiaoSong, If you agree with this, Can you make the v2 patch ?
>
>Thanks.
>>
>> David
>>