在 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.
Could you give me more help about the fix tag.
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
