Merge tag '6.4-rc5-smb3-server-fixes' of git://git.samba.org/ksmbd

Pull smb server fixes from Steve French:
 "Five smb3 server fixes, all also for stable:

   - Fix four slab out of bounds warnings: improve checks for protocol
     id, and for small packet length, and for create context parsing,
     and for negotiate context parsing

   - Fix for incorrect dereferencing POSIX ACLs"

* tag '6.4-rc5-smb3-server-fixes' of git://git.samba.org/ksmbd:
  ksmbd: validate smb request protocol id
  ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loop
  ksmbd: fix posix_acls and acls dereferencing possible ERR_PTR()
  ksmbd: fix out-of-bound read in parse_lease_state()
  ksmbd: fix out-of-bound read in deassemble_neg_contexts()

diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c
index 4882a81..2a717d1 100644 (file)
--- a/fs/smb/server/connection.c
+++ b/fs/smb/server/connection.c
@@ -294,6 +294,9 @@ bool ksmbd_conn_alive(struct ksmbd_conn *conn)
        return true;
 }
 
+#define SMB1_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb_hdr))
+#define SMB2_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr) + 4)
+
 /**
  * ksmbd_conn_handler_loop() - session thread to listen on new smb requests
  * @p:         connection instance
@@ -350,6 +353,9 @@ int ksmbd_conn_handler_loop(void *p)
                if (pdu_size > MAX_STREAM_PROT_LEN)
                        break;
 
+               if (pdu_size < SMB1_MIN_SUPPORTED_HEADER_SIZE)
+                       break;
+
                /* 4 for rfc1002 length field */
                /* 1 for implied bcc[0] */
                size = pdu_size + 4 + 1;
@@ -358,8 +364,6 @@ int ksmbd_conn_handler_loop(void *p)
                        break;
 
                memcpy(conn->request_buf, hdr_buf, sizeof(hdr_buf));
-               if (!ksmbd_smb_request(conn))
-                       break;
 
                /*
                 * We already read 4 bytes to find out PDU size, now
@@ -377,6 +381,15 @@ int ksmbd_conn_handler_loop(void *p)
                        continue;
                }
 
+               if (!ksmbd_smb_request(conn))
+                       break;
+
+               if (((struct smb2_hdr *)smb2_get_msg(conn->request_buf))->ProtocolId ==
+                   SMB2_PROTO_NUMBER) {
+                       if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
+                               break;
+               }
+
                if (!default_conn_ops.process_fn) {
                        pr_err("No connection request callback\n");
                        break;

diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
index db181bd..844b303 100644 (file)
--- a/fs/smb/server/oplock.c
+++ b/fs/smb/server/oplock.c
@@ -1415,56 +1415,38 @@ void create_lease_buf(u8 *rbuf, struct lease *lease)
  */
 struct lease_ctx_info *parse_lease_state(void *open_req)
 {
-       char *data_offset;
        struct create_context *cc;
-       unsigned int next = 0;
-       char *name;
-       bool found = false;
        struct smb2_create_req *req = (struct smb2_create_req *)open_req;
-       struct lease_ctx_info *lreq = kzalloc(sizeof(struct lease_ctx_info),
-               GFP_KERNEL);
+       struct lease_ctx_info *lreq;
+
+       cc = smb2_find_context_vals(req, SMB2_CREATE_REQUEST_LEASE, 4);
+       if (IS_ERR_OR_NULL(cc))
+               return NULL;
+
+       lreq = kzalloc(sizeof(struct lease_ctx_info), GFP_KERNEL);
        if (!lreq)
                return NULL;
 
-       data_offset = (char *)req + le32_to_cpu(req->CreateContextsOffset);
-       cc = (struct create_context *)data_offset;
-       do {
-               cc = (struct create_context *)((char *)cc + next);
-               name = le16_to_cpu(cc->NameOffset) + (char *)cc;
-               if (le16_to_cpu(cc->NameLength) != 4 ||
-                   strncmp(name, SMB2_CREATE_REQUEST_LEASE, 4)) {
-                       next = le32_to_cpu(cc->Next);
-                       continue;
-               }
-               found = true;
-               break;
-       } while (next != 0);
+       if (sizeof(struct lease_context_v2) == le32_to_cpu(cc->DataLength)) {
+               struct create_lease_v2 *lc = (struct create_lease_v2 *)cc;
 
-       if (found) {
-               if (sizeof(struct lease_context_v2) == le32_to_cpu(cc->DataLength)) {
-                       struct create_lease_v2 *lc = (struct create_lease_v2 *)cc;
-
-                       memcpy(lreq->lease_key, lc->lcontext.LeaseKey, SMB2_LEASE_KEY_SIZE);
-                       lreq->req_state = lc->lcontext.LeaseState;
-                       lreq->flags = lc->lcontext.LeaseFlags;
-                       lreq->duration = lc->lcontext.LeaseDuration;
-                       memcpy(lreq->parent_lease_key, lc->lcontext.ParentLeaseKey,
-                              SMB2_LEASE_KEY_SIZE);
-                       lreq->version = 2;
-               } else {
-                       struct create_lease *lc = (struct create_lease *)cc;
+               memcpy(lreq->lease_key, lc->lcontext.LeaseKey, SMB2_LEASE_KEY_SIZE);
+               lreq->req_state = lc->lcontext.LeaseState;
+               lreq->flags = lc->lcontext.LeaseFlags;
+               lreq->duration = lc->lcontext.LeaseDuration;
+               memcpy(lreq->parent_lease_key, lc->lcontext.ParentLeaseKey,
+                               SMB2_LEASE_KEY_SIZE);
+               lreq->version = 2;
+       } else {
+               struct create_lease *lc = (struct create_lease *)cc;
 
-                       memcpy(lreq->lease_key, lc->lcontext.LeaseKey, SMB2_LEASE_KEY_SIZE);
-                       lreq->req_state = lc->lcontext.LeaseState;
-                       lreq->flags = lc->lcontext.LeaseFlags;
-                       lreq->duration = lc->lcontext.LeaseDuration;
-                       lreq->version = 1;
-               }
-               return lreq;
+               memcpy(lreq->lease_key, lc->lcontext.LeaseKey, SMB2_LEASE_KEY_SIZE);
+               lreq->req_state = lc->lcontext.LeaseState;
+               lreq->flags = lc->lcontext.LeaseFlags;
+               lreq->duration = lc->lcontext.LeaseDuration;
+               lreq->version = 1;
        }
-
-       kfree(lreq);
-       return NULL;
+       return lreq;
 }
 
 /**

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 7a81541..25c0ba0 100644 (file)
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -963,13 +963,13 @@ static void decode_sign_cap_ctxt(struct ksmbd_conn *conn,
 
 static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
                                      struct smb2_negotiate_req *req,
-                                     int len_of_smb)
+                                     unsigned int len_of_smb)
 {
        /* +4 is to account for the RFC1001 len field */
        struct smb2_neg_context *pctx = (struct smb2_neg_context *)req;
        int i = 0, len_of_ctxts;
-       int offset = le32_to_cpu(req->NegotiateContextOffset);
-       int neg_ctxt_cnt = le16_to_cpu(req->NegotiateContextCount);
+       unsigned int offset = le32_to_cpu(req->NegotiateContextOffset);
+       unsigned int neg_ctxt_cnt = le16_to_cpu(req->NegotiateContextCount);
        __le32 status = STATUS_INVALID_PARAMETER;
 
        ksmbd_debug(SMB, "decoding %d negotiate contexts\n", neg_ctxt_cnt);
@@ -983,7 +983,7 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
        while (i++ < neg_ctxt_cnt) {
                int clen, ctxt_len;
 
-               if (len_of_ctxts < sizeof(struct smb2_neg_context))
+               if (len_of_ctxts < (int)sizeof(struct smb2_neg_context))
                        break;
 
                pctx = (struct smb2_neg_context *)((char *)pctx + offset);
@@ -1038,9 +1038,8 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
                }
 
                /* offsets must be 8 byte aligned */
-               clen = (clen + 7) & ~0x7;
-               offset = clen + sizeof(struct smb2_neg_context);
-               len_of_ctxts -= clen + sizeof(struct smb2_neg_context);
+               offset = (ctxt_len + 7) & ~0x7;
+               len_of_ctxts -= offset;
        }
        return status;
 }

diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
index af0c2a9..569e5ee 100644 (file)
--- a/fs/smb/server/smb_common.c
+++ b/fs/smb/server/smb_common.c
@@ -158,7 +158,19 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
  */
 bool ksmbd_smb_request(struct ksmbd_conn *conn)
 {
-       return conn->request_buf[0] == 0;
+       __le32 *proto = (__le32 *)smb2_get_msg(conn->request_buf);
+
+       if (*proto == SMB2_COMPRESSION_TRANSFORM_ID) {
+               pr_err_ratelimited("smb2 compression not support yet");
+               return false;
+       }
+
+       if (*proto != SMB1_PROTO_NUMBER &&
+           *proto != SMB2_PROTO_NUMBER &&
+           *proto != SMB2_TRANSFORM_PROTO_NUM)
+               return false;
+
+       return true;
 }
 
 static bool supported_protocol(int idx)

diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index 6d6cfb6..0a5862a 100644 (file)
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1290,7 +1290,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
 
        if (IS_ENABLED(CONFIG_FS_POSIX_ACL)) {
                posix_acls = get_inode_acl(d_inode(path->dentry), ACL_TYPE_ACCESS);
-               if (posix_acls && !found) {
+               if (!IS_ERR_OR_NULL(posix_acls) && !found) {
                        unsigned int id = -1;
 
                        pa_entry = posix_acls->a_entries;
@@ -1314,7 +1314,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
                                }
                        }
                }
-               if (posix_acls)
+               if (!IS_ERR_OR_NULL(posix_acls))
                        posix_acl_release(posix_acls);
        }
 

diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 6f30291..f9fb778 100644 (file)
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -1321,7 +1321,7 @@ static struct xattr_smb_acl *ksmbd_vfs_make_xattr_posix_acl(struct mnt_idmap *id
                return NULL;
 
        posix_acls = get_inode_acl(inode, acl_type);
-       if (!posix_acls)
+       if (IS_ERR_OR_NULL(posix_acls))
                return NULL;
 
        smb_acl = kzalloc(sizeof(struct xattr_smb_acl) +
@@ -1830,7 +1830,7 @@ int ksmbd_vfs_inherit_posix_acl(struct mnt_idmap *idmap,
                return -EOPNOTSUPP;
 
        acls = get_inode_acl(parent_inode, ACL_TYPE_DEFAULT);
-       if (!acls)
+       if (IS_ERR_OR_NULL(acls))
                return -ENOENT;
        pace = acls->a_entries;