Re: [PATCH v4 2/5] ksmbd: Remove the wrong message length check of FSCTL_VALIDATE_NEGOTIATE_INFO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2022-09-05 11:29 GMT+09:00, zhangxiaoxu (A) <zhangxiaoxu5@xxxxxxxxxx>:
>
>
> 在 2022/9/2 22:47, Namjae Jeon 写道:
>> 2022-09-02 22:28 GMT+09:00, Tom Talpey <tom@xxxxxxxxxx>:
>>> On 9/1/2022 10:24 AM, Zhang Xiaoxu wrote:
>>>> The struct validate_negotiate_info_req change from variable-length
>>>> array
>>>> to reguler array, but the message length check is unchanged.
>>>>
>>>> The fsctl_validate_negotiate_info() already check the message length,
>>>> so
>>>> remove it from smb2_ioctl().
>>>>
>>>> Fixes: c7803b05f74b ("smb3: fix ksmbd bigendian bug in oplock break,
>>>> and
>> I think the fixes tag is wrong. Isn't the below correct?
>> Fixes: f7db8fd03a4bc ("ksmbd: add validation in smb2_ioctl")
>
> Before commit c7803b05f74b ("smb3: fix ksmbd bigendian bug in oplock break,
> and
> move its struct to smbfs_common"), the struct defined in
> fs/ksmbd/smb2pdu.h:
>
>    struct validate_negotiate_info_req {
>           __le32 Capabilities;
>           __u8   Guid[SMB2_CLIENT_GUID_SIZE];
>           __le16 SecurityMode;
>           __le16 DialectCount;
>           __le16 Dialects[1]; /* dialect (someday maybe list) client asked
> for */
>    } __packed;
>
> After this commit, the struct defined in fs/smbfs_common/smb2pdu.h:
>    struct validate_negotiate_info_req {
>           __le32 Capabilities;
>           __u8   Guid[SMB2_CLIENT_GUID_SIZE];
>           __le16 SecurityMode;
>           __le16 DialectCount;
>           __le16 Dialects[4]; /* BB expand this if autonegotiate > 4
> dialects */
>    } __packed;
>
> The 'Dialects' array length from 1 change to 4.
Okay, Do you think that your patch is not needed without commit c7803b05f74b ?
I understood that the InputCount check doesn't take the DialectCount
into account and it's already a duplicate check, So you try to remove
that.

>
> Before commit c7803b05f74b, the message should contain at lease 1 dialects,
> but after this commit, it changed to should contain at lease 4 dialects.
>
> So the fixes tag should be c7803b05f74b.




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux