2022-09-16 20:20 GMT+09:00, zhangxiaoxu (A) <zhangxiaoxu5@xxxxxxxxxx>:
>
>
> 在 2022/9/16 8:26, Namjae Jeon 写道:
>> 2022-09-14 11:17 GMT+09:00, Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx>:
>>> The structure size includes 4 dialect slots, but the protocol does not
>>> require the client to send all 4. So this allows the negotiation to not
>>> fail.
>>>
>>> Fixes: c7803b05f74b ("smb3: fix ksmbd bigendian bug in oplock break, and
>>> move its struct to smbfs_common")
>> NACK. I am still thinking this tag is wrong.
> Thanks for your comments.
>
> In the v2, I remove the validation expression, and as your comments
> in v2, duplicate check is unneed.
>
> I add it back in v6 because tom's comments, we should ensure the req
> has 'DialectCount', before validate the req has enough 'Dialects',
> otherwise, dereferencing 'neg_req->DialectCount' will be OOB read.
>
> Maybe the validation expression as below much better:
> @@ -7640,7 +7640,8 @@ int smb2_ioctl(struct ksmbd_work *work)
> ...
> if (in_buf_len < offsetof(struct validate_negotiate_info_req,
> Dialects[1]))
>
> If even there's no one Dialects, 'the fsctl_validate_negotiate_info'
> still return -EINVAL:
>
> fsctl_validate_negotiate_info
> ksmbd_lookup_dialect_by_id(neg_req->DialectCount=0)
> return BAD_PROT_ID
> ret = -EINVAL;
>
> Same as before commit c7803b05f74b.
Sorry, I don't understand what you say.
>
> Could you give me more help about the fix tag.
Please change a tag to commit f7db8fd03a4bc in this patch.
Thanks.
>
> Thanks.
> Zhang Xiaoxu
>>
>>> Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx>
>>> Cc: <stable@xxxxxxxxxxxxxxx>
>>> ---
>>> fs/ksmbd/smb2pdu.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>>> index b56d7688ccf1..09ae601e64f9 100644
>>> --- a/fs/ksmbd/smb2pdu.c
>>> +++ b/fs/ksmbd/smb2pdu.c
>>> @@ -7640,7 +7640,8 @@ int smb2_ioctl(struct ksmbd_work *work)
>>> goto out;
>>> }
>>>
>>> - if (in_buf_len < sizeof(struct validate_negotiate_info_req)) {
>>> + if (in_buf_len < offsetof(struct validate_negotiate_info_req,
>>> + Dialects)) {
>>> ret = -EINVAL;
>>> goto out;
>>> }
>>> --
>>> 2.31.1
>>>
>>>
>
