| Message ID | 20220831075255.2667077-1-zhangxiaoxu5@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix some bug in FSCTL_VALIDATE_NEGOTIATE_INFO handler | expand |
On 8/31/2022 3:52 AM, Zhang Xiaoxu wrote: > v1->v2: fix some bug in ksmbd when handle FSCTL_VALIDATE_NEGOTIATE_INFO > message > > Zhang Xiaoxu (3): > cifs: Fix the error length of VALIDATE_NEGOTIATE_INFO message > ksmbd: Remove the wrong message length check of > FSCTL_VALIDATE_NEGOTIATE_INFO > ksmbd: Fix wrong return value in smb2_ioctl() when wrong out_buf_len > > fs/cifs/smb2pdu.c | 4 ++-- > fs/ksmbd/smb2pdu.c | 9 ++++----- > 2 files changed, 6 insertions(+), 7 deletions(-) > Sorry but these are a NAK from me - they don't actually change the definition to a variable-length array, they just attempt to undo the broken "4", in several places. The real fix begins in smbpdu.h in this line: __le16 Dialects[4]; --> Dialects[] Also, the change to ksmbd is incorrect, it is critical to check that the inbound buffer holds at least enough data to be able to dereference the DialectCount, followed by a second check that all the counted array elements are present. Also that the outbound buffer is large enough to return a single dialect. Tom.
在 2022/8/31 22:50, Tom Talpey 写道: > On 8/31/2022 3:52 AM, Zhang Xiaoxu wrote: >> v1->v2: fix some bug in ksmbd when handle FSCTL_VALIDATE_NEGOTIATE_INFO >> message >> >> Zhang Xiaoxu (3): >> cifs: Fix the error length of VALIDATE_NEGOTIATE_INFO message >> ksmbd: Remove the wrong message length check of >> FSCTL_VALIDATE_NEGOTIATE_INFO >> ksmbd: Fix wrong return value in smb2_ioctl() when wrong out_buf_len >> >> fs/cifs/smb2pdu.c | 4 ++-- >> fs/ksmbd/smb2pdu.c | 9 ++++----- >> 2 files changed, 6 insertions(+), 7 deletions(-) >> > > Sorry but these are a NAK from me - they don't actually change > the definition to a variable-length array, they just attempt > to undo the broken "4", in several places. The real fix begins > in smbpdu.h in this line: > __le16 Dialects[4]; --> Dialects[] This patchset just for fix the problems, the patches for refactor to variable-length array is on the way. > > Also, the change to ksmbd is incorrect, it is critical to check > that the inbound buffer holds at least enough data to be able > to dereference the DialectCount, followed by a second check > that all the counted array elements are present. Also that > the outbound buffer is large enough to return a single dialect. The 'fsctl_validate_negotiate_info' function already check the inbound buffer length, so remove the wrong inbound check in 'smb2_ioctl', do you mean move the inbound check from 'fsctl_validate_negotiate_info' to 'smb2_ioctl'? 7387 static int fsctl_validate_negotiate_info(struct ksmbd_conn *conn, │ ksmbd_spnego_negtokeninit.as 7388 struct validate_negotiate_info_req *neg_req, │ ksmbd_spnego_negtokeninit.as 7389 struct validate_negotiate_info_rsp *neg_rsp, │ ksmbd_spnego_negtokentarg.as 7390 unsigned int in_buf_len) │ ksmbd_spnego_negtokentarg.as 7391 { │ ksmbd_spnego_negtokentarg.as 7392 int ret = 0; │ ksmbd_spnego_negtokentarg.as 7393 int dialect; │ ksmbd_work.c 7394 │ ksmbd_work.h 7395 if (in_buf_len < offsetof(struct validate_negotiate_info_req, Dialects) + │ ksmbd_work.o 7396 le16_to_cpu(neg_req->DialectCount) * sizeof(__le16)) │ Makefile 7397 return -EINVAL; > > Tom.
v1->v2: fix some bug in ksmbd when handle FSCTL_VALIDATE_NEGOTIATE_INFO message Zhang Xiaoxu (3): cifs: Fix the error length of VALIDATE_NEGOTIATE_INFO message ksmbd: Remove the wrong message length check of FSCTL_VALIDATE_NEGOTIATE_INFO ksmbd: Fix wrong return value in smb2_ioctl() when wrong out_buf_len fs/cifs/smb2pdu.c | 4 ++-- fs/ksmbd/smb2pdu.c | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-)