* [PATCH v7 0/9] Buffer validation and compound handling patches
@ 2021-10-05 5:03 Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 1/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_verify_smb_message() Ralph Boehme
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs; +Cc: Ralph Boehme
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)
v5:
- remove PDU size validation from ksmbd_conn_handler_loop()
- add PDU size validation to ksmbd_smb2_check_message()
- fix compound non-related request handling
v6:
- check we can access ProtocolId in ksmbd_verify_smb_message()
- optimize tcon and session check functions for compound related PDUs
- drop patch that broke SMB1 negprot
- check credits after fully validating PDU size
v7:
- drop header size check in ksmbd_verify_smb_message()
- fix invalid read when accessing StructureSize2 in
ksmbd_smb2_check_message()
- validate credit charge after validating SMB2 PDU body size
Ralph Boehme (9):
ksmbd: use ksmbd_req_buf_next() in ksmbd_verify_smb_message()
ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()
ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() in
ksmbd_smb2_check_message()
ksmbd: check buffer is big enough to access the SMB2 PUD body size
field
ksmdb: validate credit charge after validating SMB2 PDU body size
ksmdb: use cmd helper variable in smb2_get_ksmbd_tcon()
ksmdb: make smb2_get_ksmbd_tcon() callable with chained PDUs
ksmbd: make smb2_check_user_session() callable for compound PDUs
ksmdb: move session and tcon validation to __process_request()
fs/ksmbd/ksmbd_work.h | 1 +
fs/ksmbd/server.c | 46 +++++++++++++++++++++-------------
fs/ksmbd/smb2misc.c | 58 +++++++++++++++++++++++++++----------------
fs/ksmbd/smb2pdu.c | 39 +++++++++++++++++++++++------
fs/ksmbd/smb2pdu.h | 1 +
fs/ksmbd/smb_common.c | 2 +-
6 files changed, 101 insertions(+), 46 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 1/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_verify_smb_message()
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
2021-10-05 7:26 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 2/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message() Ralph Boehme
` (7 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs; +Cc: Ralph Boehme
No change in behaviour.
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/smb_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
index 707490ab1f4c..e1e5a071678e 100644
--- a/fs/ksmbd/smb_common.c
+++ b/fs/ksmbd/smb_common.c
@@ -132,7 +132,7 @@ int ksmbd_lookup_protocol_idx(char *str)
*/
int ksmbd_verify_smb_message(struct ksmbd_work *work)
{
- struct smb2_hdr *smb2_hdr = work->request_buf + work->next_smb2_rcv_hdr_off;
+ struct smb2_hdr *smb2_hdr = ksmbd_req_buf_next(work);
struct smb_hdr *hdr;
if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 2/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 1/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_verify_smb_message() Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
2021-10-05 7:27 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 3/9] ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() " Ralph Boehme
` (6 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs
Cc: Ralph Boehme, Namjae Jeon, Tom Talpey, Ronnie Sahlberg,
Steve French, Hyunchul Lee
No change in behaviour.
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/smb2misc.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index 9edd9c161b27..2cc031c39514 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -329,17 +329,12 @@ static int smb2_validate_credit_charge(struct smb2_hdr *hdr)
int ksmbd_smb2_check_message(struct ksmbd_work *work)
{
- struct smb2_pdu *pdu = work->request_buf;
+ struct smb2_pdu *pdu = ksmbd_req_buf_next(work);
struct smb2_hdr *hdr = &pdu->hdr;
int command;
__u32 clc_len; /* calculated length */
__u32 len = get_rfc1002_len(pdu);
- if (work->next_smb2_rcv_hdr_off) {
- pdu = ksmbd_req_buf_next(work);
- hdr = &pdu->hdr;
- }
-
if (le32_to_cpu(hdr->NextCommand) > 0) {
len = le32_to_cpu(hdr->NextCommand);
} else if (work->next_smb2_rcv_hdr_off) {
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 3/9] ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() in ksmbd_smb2_check_message()
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 1/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_verify_smb_message() Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 2/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message() Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
2021-10-05 7:29 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 4/9] ksmbd: check buffer is big enough to access the SMB2 PUD body size field Ralph Boehme
` (5 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs
Cc: Ralph Boehme, Namjae Jeon, Tom Talpey, Ronnie Sahlberg,
Steve French, Hyunchul Lee
No change in behaviour.
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/smb2misc.c | 36 +++++++++++++++++++++++++++---------
fs/ksmbd/smb2pdu.h | 1 +
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index 2cc031c39514..7ed266eb6c5e 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -333,14 +333,7 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
struct smb2_hdr *hdr = &pdu->hdr;
int command;
__u32 clc_len; /* calculated length */
- __u32 len = get_rfc1002_len(pdu);
-
- if (le32_to_cpu(hdr->NextCommand) > 0) {
- len = le32_to_cpu(hdr->NextCommand);
- } else if (work->next_smb2_rcv_hdr_off) {
- len -= work->next_smb2_rcv_hdr_off;
- len = round_up(len, 8);
- }
+ __u32 len = ksmbd_smb2_cur_pdu_buflen(work);
if (check_smb2_hdr(hdr))
return 1;
@@ -395,7 +388,7 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
* Some windows servers (win2016) will pad also the final
* PDU in a compound to 8 bytes.
*/
- if (ALIGN(clc_len, 8) == len)
+ if (ALIGN(clc_len, 8) == ALIGN(len, 8))
return 0;
/*
@@ -427,3 +420,28 @@ int smb2_negotiate_request(struct ksmbd_work *work)
{
return ksmbd_smb_negotiate_common(work, SMB2_NEGOTIATE_HE);
}
+
+/**
+ * ksmbd_smb2_cur_pdu_buflen() - Get len of current SMB2 PDU buffer
+ * This returns the lenght including any possible padding.
+ * @work: smb work containing request buffer
+ */
+unsigned int ksmbd_smb2_cur_pdu_buflen(struct ksmbd_work *work)
+{
+ struct smb2_hdr *hdr = ksmbd_req_buf_next(work);
+ unsigned int buf_len;
+ unsigned int pdu_len;
+
+ if (hdr->NextCommand != 0) {
+ /*
+ * hdr->NextCommand has already been validated by
+ * init_chained_smb2_rsp().
+ */
+ return __le32_to_cpu(hdr->NextCommand);
+ }
+
+ buf_len = get_rfc1002_len(work->request_buf);
+ pdu_len = buf_len - work->next_smb2_rcv_hdr_off;
+ return pdu_len;
+}
+
diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h
index a6dec5ec6a54..c5fa8256b0bb 100644
--- a/fs/ksmbd/smb2pdu.h
+++ b/fs/ksmbd/smb2pdu.h
@@ -1680,6 +1680,7 @@ int smb2_set_rsp_credits(struct ksmbd_work *work);
/* smb2 misc functions */
int ksmbd_smb2_check_message(struct ksmbd_work *work);
+unsigned int ksmbd_smb2_cur_pdu_buflen(struct ksmbd_work *work);
/* smb2 command handlers */
int smb2_handle_negotiate(struct ksmbd_work *work);
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 4/9] ksmbd: check buffer is big enough to access the SMB2 PUD body size field
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
` (2 preceding siblings ...)
2021-10-05 5:03 ` [PATCH v7 3/9] ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() " Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 5/9] ksmdb: validate credit charge after validating SMB2 PDU body size Ralph Boehme
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs
Cc: Ralph Boehme, Namjae Jeon, Tom Talpey, Ronnie Sahlberg,
Steve French, Hyunchul Lee
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/smb2misc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index 7ed266eb6c5e..50521b5a50b5 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -350,6 +350,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
return 1;
}
+ if (len < sizeof(struct smb2_pdu) - 4)
+ return 1;
+
if (smb2_req_struct_sizes[command] != pdu->StructureSize2) {
if (command != SMB2_OPLOCK_BREAK_HE &&
(hdr->Status == 0 || pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE)) {
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 5/9] ksmdb: validate credit charge after validating SMB2 PDU body size
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
` (3 preceding siblings ...)
2021-10-05 5:03 ` [PATCH v7 4/9] ksmbd: check buffer is big enough to access the SMB2 PUD body size field Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
2021-10-05 7:58 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 6/9] ksmdb: use cmd helper variable in smb2_get_ksmbd_tcon() Ralph Boehme
` (3 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs; +Cc: Ralph Boehme
smb2_validate_credit_charge() accesses fields in the SMB2 PDU body, but until
smb2_calc_size() is called the PDU has not yet been verified to be large enough
to access the PDU dynamic part length field.
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/smb2misc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index 50521b5a50b5..1f14120a0e48 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -373,12 +373,6 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
}
}
- if ((work->conn->vals->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU) &&
- smb2_validate_credit_charge(hdr)) {
- work->conn->ops->set_rsp_status(work, STATUS_INVALID_PARAMETER);
- return 1;
- }
-
if (smb2_calc_size(hdr, &clc_len))
return 1;
@@ -416,6 +410,12 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
return 1;
}
+ if ((work->conn->vals->capabilities & SMB2_GLOBAL_CAP_LARGE_MTU) &&
+ smb2_validate_credit_charge(hdr)) {
+ work->conn->ops->set_rsp_status(work, STATUS_INVALID_PARAMETER);
+ return 1;
+ }
+
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 6/9] ksmdb: use cmd helper variable in smb2_get_ksmbd_tcon()
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
` (4 preceding siblings ...)
2021-10-05 5:03 ` [PATCH v7 5/9] ksmdb: validate credit charge after validating SMB2 PDU body size Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
2021-10-05 7:59 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 7/9] ksmdb: make smb2_get_ksmbd_tcon() callable with chained PDUs Ralph Boehme
` (2 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs
Cc: Ralph Boehme, Namjae Jeon, Tom Talpey, Ronnie Sahlberg,
Steve French, Hyunchul Lee
No change in behaviour.
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/smb2pdu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index ed8324f9c2bd..e10ddc1fce09 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -94,12 +94,13 @@ struct channel *lookup_chann_list(struct ksmbd_session *sess, struct ksmbd_conn
int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
{
struct smb2_hdr *req_hdr = work->request_buf;
+ unsigned int cmd = le16_to_cpu(req_hdr->Command);
int tree_id;
work->tcon = NULL;
- if (work->conn->ops->get_cmd_val(work) == SMB2_TREE_CONNECT_HE ||
- work->conn->ops->get_cmd_val(work) == SMB2_CANCEL_HE ||
- work->conn->ops->get_cmd_val(work) == SMB2_LOGOFF_HE) {
+ if (cmd == SMB2_TREE_CONNECT_HE ||
+ cmd == SMB2_CANCEL_HE ||
+ cmd == SMB2_LOGOFF_HE) {
ksmbd_debug(SMB, "skip to check tree connect request\n");
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 7/9] ksmdb: make smb2_get_ksmbd_tcon() callable with chained PDUs
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
` (5 preceding siblings ...)
2021-10-05 5:03 ` [PATCH v7 6/9] ksmdb: use cmd helper variable in smb2_get_ksmbd_tcon() Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 8/9] ksmbd: make smb2_check_user_session() callable for compound PDUs Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 9/9] ksmdb: move session and tcon validation to __process_request() Ralph Boehme
8 siblings, 0 replies; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs
Cc: Ralph Boehme, Namjae Jeon, Tom Talpey, Ronnie Sahlberg,
Steve French, Hyunchul Lee
Also track the tcon id of compound requests.
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/ksmbd_work.h | 1 +
fs/ksmbd/smb2pdu.c | 14 +++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/fs/ksmbd/ksmbd_work.h b/fs/ksmbd/ksmbd_work.h
index f7156bc50049..91363d508909 100644
--- a/fs/ksmbd/ksmbd_work.h
+++ b/fs/ksmbd/ksmbd_work.h
@@ -46,6 +46,7 @@ struct ksmbd_work {
u64 compound_fid;
u64 compound_pfid;
u64 compound_sid;
+ u32 compound_tid;
const struct cred *saved_cred;
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index e10ddc1fce09..1755a524beb3 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -97,7 +97,6 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
unsigned int cmd = le16_to_cpu(req_hdr->Command);
int tree_id;
- work->tcon = NULL;
if (cmd == SMB2_TREE_CONNECT_HE ||
cmd == SMB2_CANCEL_HE ||
cmd == SMB2_LOGOFF_HE) {
@@ -110,13 +109,26 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
return -ENOENT;
}
+ if (req_hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS) {
+ if (!work->tcon) {
+ pr_err("Missing tcon\n");
+ return -EINVAL;
+ }
+ return 1;
+ }
+
+ work->tcon = NULL;
+ work->compound_tid = 0;
+
tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
+
work->tcon = ksmbd_tree_conn_lookup(work->sess, tree_id);
if (!work->tcon) {
pr_err("Invalid tid %d\n", tree_id);
return -EINVAL;
}
+ work->compound_tid = tree_id;
return 1;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 8/9] ksmbd: make smb2_check_user_session() callable for compound PDUs
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
` (6 preceding siblings ...)
2021-10-05 5:03 ` [PATCH v7 7/9] ksmdb: make smb2_get_ksmbd_tcon() callable with chained PDUs Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 9/9] ksmdb: move session and tcon validation to __process_request() Ralph Boehme
8 siblings, 0 replies; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs
Cc: Ralph Boehme, Namjae Jeon, Tom Talpey, Ronnie Sahlberg,
Steve French, Hyunchul Lee
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/smb2pdu.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 1755a524beb3..c137c1a94b99 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -416,7 +416,6 @@ static void init_chained_smb2_rsp(struct ksmbd_work *work)
work->compound_pfid =
le64_to_cpu(((struct smb2_create_rsp *)rsp)->
PersistentFileId);
- work->compound_sid = le64_to_cpu(rsp->SessionId);
}
len = get_rfc1002_len(work->response_buf) - work->next_smb2_rsp_hdr_off;
@@ -596,7 +595,6 @@ int smb2_check_user_session(struct ksmbd_work *work)
unsigned int cmd = conn->ops->get_cmd_val(work);
unsigned long long sess_id;
- work->sess = NULL;
/*
* SMB2_ECHO, SMB2_NEGOTIATE, SMB2_SESSION_SETUP command do not
* require a session id, so no need to validate user session's for
@@ -609,11 +607,25 @@ int smb2_check_user_session(struct ksmbd_work *work)
if (!ksmbd_conn_good(work))
return -EINVAL;
+ if (req_hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS) {
+ if (work->sess) {
+ pr_err("Missing session\n");
+ return -EINVAL;
+ }
+ return 1;
+ }
+
+ work->sess = NULL;
+ work->compound_sid = 0;
+
sess_id = le64_to_cpu(req_hdr->SessionId);
+
/* Check for validity of user session */
work->sess = ksmbd_session_lookup_all(conn, sess_id);
- if (work->sess)
+ if (work->sess) {
+ work->compound_sid = sess_id;
return 1;
+ }
ksmbd_debug(SMB, "Invalid user session, Uid %llu\n", sess_id);
return -EINVAL;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 9/9] ksmdb: move session and tcon validation to __process_request()
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
` (7 preceding siblings ...)
2021-10-05 5:03 ` [PATCH v7 8/9] ksmbd: make smb2_check_user_session() callable for compound PDUs Ralph Boehme
@ 2021-10-05 5:03 ` Ralph Boehme
8 siblings, 0 replies; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 5:03 UTC (permalink / raw)
To: linux-cifs
Cc: Ralph Boehme, Namjae Jeon, Tom Talpey, Ronnie Sahlberg,
Steve French, Hyunchul Lee
For compound non-related operations session id and tree id must be taken from
earch PDU.
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
---
fs/ksmbd/server.c | 46 +++++++++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 17 deletions(-)
diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c
index 2a2b2135bfde..5d1ef277653f 100644
--- a/fs/ksmbd/server.c
+++ b/fs/ksmbd/server.c
@@ -101,6 +101,32 @@ static inline int check_conn_state(struct ksmbd_work *work)
return 0;
}
+static int check_session_and_tcon(struct ksmbd_work *work)
+{
+ int rc;
+
+ if (work->conn->ops->check_user_session == NULL)
+ return 0;
+
+ rc = work->conn->ops->check_user_session(work);
+ if (rc < 0) {
+ work->conn->ops->set_rsp_status(work,
+ STATUS_USER_SESSION_DELETED);
+ return 1;
+ }
+ if (rc == 0)
+ return 0;
+
+ rc = work->conn->ops->get_ksmbd_tcon(work);
+ if (rc < 0) {
+ work->conn->ops->set_rsp_status(work,
+ STATUS_NETWORK_NAME_DELETED);
+ return 1;
+ }
+
+ return 0;
+}
+
#define SERVER_HANDLER_CONTINUE 0
#define SERVER_HANDLER_ABORT 1
@@ -117,6 +143,9 @@ static int __process_request(struct ksmbd_work *work, struct ksmbd_conn *conn,
if (ksmbd_verify_smb_message(work))
return SERVER_HANDLER_ABORT;
+ if (check_session_and_tcon(work))
+ return SERVER_HANDLER_ABORT;
+
command = conn->ops->get_cmd_val(work);
*cmd = command;
@@ -184,23 +213,6 @@ static void __handle_ksmbd_work(struct ksmbd_work *work,
goto send;
}
- if (conn->ops->check_user_session) {
- rc = conn->ops->check_user_session(work);
- if (rc < 0) {
- command = conn->ops->get_cmd_val(work);
- conn->ops->set_rsp_status(work,
- STATUS_USER_SESSION_DELETED);
- goto send;
- } else if (rc > 0) {
- rc = conn->ops->get_ksmbd_tcon(work);
- if (rc < 0) {
- conn->ops->set_rsp_status(work,
- STATUS_NETWORK_NAME_DELETED);
- goto send;
- }
- }
- }
-
do {
rc = __process_request(work, conn, &command);
if (rc == SERVER_HANDLER_ABORT)
--
2.31.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v7 1/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_verify_smb_message()
2021-10-05 5:03 ` [PATCH v7 1/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_verify_smb_message() Ralph Boehme
@ 2021-10-05 7:26 ` Namjae Jeon
0 siblings, 0 replies; 17+ messages in thread
From: Namjae Jeon @ 2021-10-05 7:26 UTC (permalink / raw)
To: Ralph Boehme; +Cc: linux-cifs
2021-10-05 14:03 GMT+09:00, Ralph Boehme <slow@samba.org>:
> No change in behaviour.
>
> Signed-off-by: Ralph Boehme <slow@samba.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks for your patch!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 2/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()
2021-10-05 5:03 ` [PATCH v7 2/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message() Ralph Boehme
@ 2021-10-05 7:27 ` Namjae Jeon
0 siblings, 0 replies; 17+ messages in thread
From: Namjae Jeon @ 2021-10-05 7:27 UTC (permalink / raw)
To: Ralph Boehme
Cc: linux-cifs, Tom Talpey, Ronnie Sahlberg, Steve French, Hyunchul Lee
2021-10-05 14:03 GMT+09:00, Ralph Boehme <slow@samba.org>:
> No change in behaviour.
>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Steve French <smfrench@gmail.com>
> Cc: Hyunchul Lee <hyc.lee@gmail.com>
> Signed-off-by: Ralph Boehme <slow@samba.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks for your patch!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 3/9] ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() in ksmbd_smb2_check_message()
2021-10-05 5:03 ` [PATCH v7 3/9] ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() " Ralph Boehme
@ 2021-10-05 7:29 ` Namjae Jeon
2021-10-05 7:46 ` Ralph Boehme
0 siblings, 1 reply; 17+ messages in thread
From: Namjae Jeon @ 2021-10-05 7:29 UTC (permalink / raw)
To: Ralph Boehme
Cc: linux-cifs, Tom Talpey, Ronnie Sahlberg, Steve French, Hyunchul Lee
2021-10-05 14:03 GMT+09:00, Ralph Boehme <slow@samba.org>:
> No change in behaviour.
>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Steve French <smfrench@gmail.com>
> Cc: Hyunchul Lee <hyc.lee@gmail.com>
> Signed-off-by: Ralph Boehme <slow@samba.org>
> ---
> fs/ksmbd/smb2misc.c | 36 +++++++++++++++++++++++++++---------
> fs/ksmbd/smb2pdu.h | 1 +
> 2 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
> index 2cc031c39514..7ed266eb6c5e 100644
> --- a/fs/ksmbd/smb2misc.c
> +++ b/fs/ksmbd/smb2misc.c
> @@ -333,14 +333,7 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
> struct smb2_hdr *hdr = &pdu->hdr;
> int command;
> __u32 clc_len; /* calculated length */
> - __u32 len = get_rfc1002_len(pdu);
> -
> - if (le32_to_cpu(hdr->NextCommand) > 0) {
> - len = le32_to_cpu(hdr->NextCommand);
> - } else if (work->next_smb2_rcv_hdr_off) {
> - len -= work->next_smb2_rcv_hdr_off;
> - len = round_up(len, 8);
> - }
> + __u32 len = ksmbd_smb2_cur_pdu_buflen(work);
>
> if (check_smb2_hdr(hdr))
> return 1;
> @@ -395,7 +388,7 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
> * Some windows servers (win2016) will pad also the final
> * PDU in a compound to 8 bytes.
> */
> - if (ALIGN(clc_len, 8) == len)
> + if (ALIGN(clc_len, 8) == ALIGN(len, 8))
Can I know why you align rfc1002 len with 8 here ?
Thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 3/9] ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() in ksmbd_smb2_check_message()
2021-10-05 7:29 ` Namjae Jeon
@ 2021-10-05 7:46 ` Ralph Boehme
2021-10-06 23:42 ` Namjae Jeon
0 siblings, 1 reply; 17+ messages in thread
From: Ralph Boehme @ 2021-10-05 7:46 UTC (permalink / raw)
To: Namjae Jeon
Cc: linux-cifs, Tom Talpey, Ronnie Sahlberg, Steve French, Hyunchul Lee
[-- Attachment #1.1: Type: text/plain, Size: 702 bytes --]
Am 05.10.21 um 09:29 schrieb Namjae Jeon:
> 2021-10-05 14:03 GMT+09:00, Ralph Boehme <slow@samba.org>:
>> - if (ALIGN(clc_len, 8) == len)
>> + if (ALIGN(clc_len, 8) == ALIGN(len, 8))
> Can I know why you align rfc1002 len with 8 here ?
this should match the previous behaviour where for compound requests we
called round_up(len, 8).
This could be done differently though:
len = ksmbd_smb2_cur_pdu_buflen(work);
if (work->next_smb2_rcv_hdr_off)
len = ALIGN(len, 8);
and then just do
if (ALIGN(clc_len, 8) == len)
-slow
--
Ralph Boehme, Samba Team https://samba.org/
SerNet Samba Team Lead https://sernet.de/en/team-samba
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 5/9] ksmdb: validate credit charge after validating SMB2 PDU body size
2021-10-05 5:03 ` [PATCH v7 5/9] ksmdb: validate credit charge after validating SMB2 PDU body size Ralph Boehme
@ 2021-10-05 7:58 ` Namjae Jeon
0 siblings, 0 replies; 17+ messages in thread
From: Namjae Jeon @ 2021-10-05 7:58 UTC (permalink / raw)
To: Ralph Boehme; +Cc: linux-cifs
2021-10-05 14:03 GMT+09:00, Ralph Boehme <slow@samba.org>:
> smb2_validate_credit_charge() accesses fields in the SMB2 PDU body, but
> until
> smb2_calc_size() is called the PDU has not yet been verified to be large
> enough
> to access the PDU dynamic part length field.
>
> Signed-off-by: Ralph Boehme <slow@samba.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks for your work!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 6/9] ksmdb: use cmd helper variable in smb2_get_ksmbd_tcon()
2021-10-05 5:03 ` [PATCH v7 6/9] ksmdb: use cmd helper variable in smb2_get_ksmbd_tcon() Ralph Boehme
@ 2021-10-05 7:59 ` Namjae Jeon
0 siblings, 0 replies; 17+ messages in thread
From: Namjae Jeon @ 2021-10-05 7:59 UTC (permalink / raw)
To: Ralph Boehme
Cc: linux-cifs, Tom Talpey, Ronnie Sahlberg, Steve French, Hyunchul Lee
2021-10-05 14:03 GMT+09:00, Ralph Boehme <slow@samba.org>:
> No change in behaviour.
>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Steve French <smfrench@gmail.com>
> Cc: Hyunchul Lee <hyc.lee@gmail.com>
> Signed-off-by: Ralph Boehme <slow@samba.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 3/9] ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() in ksmbd_smb2_check_message()
2021-10-05 7:46 ` Ralph Boehme
@ 2021-10-06 23:42 ` Namjae Jeon
0 siblings, 0 replies; 17+ messages in thread
From: Namjae Jeon @ 2021-10-06 23:42 UTC (permalink / raw)
To: Ralph Boehme
Cc: linux-cifs, Tom Talpey, Ronnie Sahlberg, Steve French, Hyunchul Lee
2021-10-05 16:46 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Am 05.10.21 um 09:29 schrieb Namjae Jeon:
>> 2021-10-05 14:03 GMT+09:00, Ralph Boehme <slow@samba.org>:
>>> - if (ALIGN(clc_len, 8) == len)
>>> + if (ALIGN(clc_len, 8) == ALIGN(len, 8))
>> Can I know why you align rfc1002 len with 8 here ?
>
> this should match the previous behaviour where for compound requests we
> called round_up(len, 8).
>
> This could be done differently though:
>
> len = ksmbd_smb2_cur_pdu_buflen(work);
> if (work->next_smb2_rcv_hdr_off)
> len = ALIGN(len, 8);
I think that this code is wrong, we don't need to add pad about last
compound *request* length.
Thanks!
>
> and then just do
>
> if (ALIGN(clc_len, 8) == len)
>
> -slow
>
> --
> Ralph Boehme, Samba Team https://samba.org/
> SerNet Samba Team Lead https://sernet.de/en/team-samba
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-10-06 23:42 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 5:03 [PATCH v7 0/9] Buffer validation and compound handling patches Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 1/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_verify_smb_message() Ralph Boehme
2021-10-05 7:26 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 2/9] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message() Ralph Boehme
2021-10-05 7:27 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 3/9] ksmbd: add and use ksmbd_smb2_cur_pdu_buflen() " Ralph Boehme
2021-10-05 7:29 ` Namjae Jeon
2021-10-05 7:46 ` Ralph Boehme
2021-10-06 23:42 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 4/9] ksmbd: check buffer is big enough to access the SMB2 PUD body size field Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 5/9] ksmdb: validate credit charge after validating SMB2 PDU body size Ralph Boehme
2021-10-05 7:58 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 6/9] ksmdb: use cmd helper variable in smb2_get_ksmbd_tcon() Ralph Boehme
2021-10-05 7:59 ` Namjae Jeon
2021-10-05 5:03 ` [PATCH v7 7/9] ksmdb: make smb2_get_ksmbd_tcon() callable with chained PDUs Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 8/9] ksmbd: make smb2_check_user_session() callable for compound PDUs Ralph Boehme
2021-10-05 5:03 ` [PATCH v7 9/9] ksmdb: move session and tcon validation to __process_request() Ralph Boehme
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).