Am 29.09.21 um 10:44 schrieb Namjae Jeon:
Cc: Tom Talpey <tom@xxxxxxxxxx>
Cc: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
Cc: Ralph Böhme <slow@xxxxxxxxx>
Cc: Steve French <smfrench@xxxxxxxxx>
Cc: Hyunchul Lee <hyc.lee@xxxxxxxxx>
Cc: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
v2:
- update comments of smb2_get_data_area_len().
- fix wrong buffer size check in fsctl_query_iface_info_ioctl().
- fix 32bit overflow in smb2_set_info.
v3:
- add buffer check for ByteCount of smb negotiate request.
- Moved buffer check of to the top of loop to avoid unneeded behavior when
out_buf_len is smaller than network_interface_info_ioctl_rsp.
- get correct out_buf_len which doesn't exceed max stream protocol length.
- subtract single smb2_lock_element for correct buffer size check in
ksmbd_smb2_check_message().
v4:
- use work->response_sz for out_buf_len calculation in smb2_ioctl.
- move smb2_neg size check to above to validate NegotiateContextOffset
field.
- remove unneeded dialect checks in smb2_sess_setup() and
smb2_handle_negotiate().
- split smb2_set_info patch into two patches(declaring
smb2_file_basic_info and buffer check)
it looks like you dropped all my patches and didn't comment on the SQUASHES that pointed at some issues.
Did I miss anything where you explained why you did this?The changes I made imho consolidated the SMB2 PDU packet size checking logic. With your changes the check for valid SMB2 PDU sizes of compound offsets is spread across the network receive layer and the compound parsing layer.
The changes I made, adding a nice helper function along the way, moved the core PDU validation into the function were it should be done: inside ksmbd_smb2_check_message().
You also dropped the fix for the possible invalid read in ksmbd_verify_smb_message() of the protocol_id field.
I might be missing something because I'm still new to the code. But generally we really sanitize the logic while we're at it now instead of adding band aids everywhere.
Thanks! -slow -- Ralph Boehme, Samba Team https://samba.org/ SerNet Samba Team Lead https://sernet.de/en/team-samba
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
