Skip to content

Commit 99ec1ed

Browse files
torvaldstorvalds
committed
Merge tag '6.4-rc6-smb3-server-fixes' of git://git.samba.org/ksmbd
Pull smb server fixes from Steve French: "Four smb3 server fixes, all also for stable: - fix potential oops in parsing compounded requests - fix various paths (mkdir, create etc) where mnt_want_write was not checked first - fix slab out of bounds in check_message and write" * tag '6.4-rc6-smb3-server-fixes' of git://git.samba.org/ksmbd: ksmbd: validate session id and tree id in the compound request ksmbd: fix out-of-bound read in smb2_write ksmbd: add mnt_want_write to ksmbd vfs functions ksmbd: validate command payload size
2 parents 692b7dc + 5005bcb commit 99ec1ed

7 files changed

Lines changed: 196 additions & 86 deletions

File tree

‎fs/smb/server/server.c‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -185,24 +185,31 @@ static void __handle_ksmbd_work(struct ksmbd_work *work,
185185
goto send;
186186
}
187187

188-
if (conn->ops->check_user_session) {
189-
rc = conn->ops->check_user_session(work);
190-
if (rc < 0) {
191-
command = conn->ops->get_cmd_val(work);
192-
conn->ops->set_rsp_status(work,
193-
STATUS_USER_SESSION_DELETED);
194-
goto send;
195-
} else if (rc > 0) {
196-
rc = conn->ops->get_ksmbd_tcon(work);
188+
do {
189+
if (conn->ops->check_user_session) {
190+
rc = conn->ops->check_user_session(work);
197191
if (rc < 0) {
198-
conn->ops->set_rsp_status(work,
199-
STATUS_NETWORK_NAME_DELETED);
192+
if (rc == -EINVAL)
193+
conn->ops->set_rsp_status(work,
194+
STATUS_INVALID_PARAMETER);
195+
else
196+
conn->ops->set_rsp_status(work,
197+
STATUS_USER_SESSION_DELETED);
200198
goto send;
199+
} else if (rc > 0) {
200+
rc = conn->ops->get_ksmbd_tcon(work);
201+
if (rc < 0) {
202+
if (rc == -EINVAL)
203+
conn->ops->set_rsp_status(work,
204+
STATUS_INVALID_PARAMETER);
205+
else
206+
conn->ops->set_rsp_status(work,
207+
STATUS_NETWORK_NAME_DELETED);
208+
goto send;
209+
}
201210
}
202211
}
203-
}
204212

205-
do {
206213
rc = __process_request(work, conn, &command);
207214
if (rc == SERVER_HANDLER_ABORT)
208215
break;

‎fs/smb/server/smb2misc.c‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -351,9 +351,16 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
351351
int command;
352352
__u32 clc_len; /* calculated length */
353353
__u32 len = get_rfc1002_len(work->request_buf);
354+
__u32 req_struct_size, next_cmd = le32_to_cpu(hdr->NextCommand);
354355

355-
if (le32_to_cpu(hdr->NextCommand) > 0)
356-
len = le32_to_cpu(hdr->NextCommand);
356+
if ((u64)work->next_smb2_rcv_hdr_off + next_cmd > len) {
357+
pr_err("next command(%u) offset exceeds smb msg size\n",
358+
next_cmd);
359+
return 1;
360+
}
361+
362+
if (next_cmd > 0)
363+
len = next_cmd;
357364
else if (work->next_smb2_rcv_hdr_off)
358365
len -= work->next_smb2_rcv_hdr_off;
359366

@@ -373,17 +380,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
373380
}
374381

375382
if (smb2_req_struct_sizes[command] != pdu->StructureSize2) {
376-
if (command != SMB2_OPLOCK_BREAK_HE &&
377-
(hdr->Status == 0 || pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE)) {
378-
/* error packets have 9 byte structure size */
379-
ksmbd_debug(SMB,
380-
"Illegal request size %u for command %d\n",
381-
le16_to_cpu(pdu->StructureSize2), command);
382-
return 1;
383-
} else if (command == SMB2_OPLOCK_BREAK_HE &&
384-
hdr->Status == 0 &&
385-
le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_20 &&
386-
le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_21) {
383+
if (command == SMB2_OPLOCK_BREAK_HE &&
384+
le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_20 &&
385+
le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_21) {
387386
/* special case for SMB2.1 lease break message */
388387
ksmbd_debug(SMB,
389388
"Illegal request size %d for oplock break\n",
@@ -392,6 +391,14 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
392391
}
393392
}
394393

394+
req_struct_size = le16_to_cpu(pdu->StructureSize2) +
395+
__SMB2_HEADER_STRUCTURE_SIZE;
396+
if (command == SMB2_LOCK_HE)
397+
req_struct_size -= sizeof(struct smb2_lock_element);
398+
399+
if (req_struct_size > len + 1)
400+
return 1;
401+
395402
if (smb2_calc_size(hdr, &clc_len))
396403
return 1;
397404

‎fs/smb/server/smb2pdu.c‎

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
9191
unsigned int cmd = le16_to_cpu(req_hdr->Command);
9292
int tree_id;
9393

94-
work->tcon = NULL;
9594
if (cmd == SMB2_TREE_CONNECT_HE ||
9695
cmd == SMB2_CANCEL_HE ||
9796
cmd == SMB2_LOGOFF_HE) {
@@ -105,10 +104,28 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
105104
}
106105

107106
tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
107+
108+
/*
109+
* If request is not the first in Compound request,
110+
* Just validate tree id in header with work->tcon->id.
111+
*/
112+
if (work->next_smb2_rcv_hdr_off) {
113+
if (!work->tcon) {
114+
pr_err("The first operation in the compound does not have tcon\n");
115+
return -EINVAL;
116+
}
117+
if (work->tcon->id != tree_id) {
118+
pr_err("tree id(%u) is different with id(%u) in first operation\n",
119+
tree_id, work->tcon->id);
120+
return -EINVAL;
121+
}
122+
return 1;
123+
}
124+
108125
work->tcon = ksmbd_tree_conn_lookup(work->sess, tree_id);
109126
if (!work->tcon) {
110127
pr_err("Invalid tid %d\n", tree_id);
111-
return -EINVAL;
128+
return -ENOENT;
112129
}
113130

114131
return 1;
@@ -547,7 +564,6 @@ int smb2_check_user_session(struct ksmbd_work *work)
547564
unsigned int cmd = conn->ops->get_cmd_val(work);
548565
unsigned long long sess_id;
549566

550-
work->sess = NULL;
551567
/*
552568
* SMB2_ECHO, SMB2_NEGOTIATE, SMB2_SESSION_SETUP command do not
553569
* require a session id, so no need to validate user session's for
@@ -558,15 +574,33 @@ int smb2_check_user_session(struct ksmbd_work *work)
558574
return 0;
559575

560576
if (!ksmbd_conn_good(conn))
561-
return -EINVAL;
577+
return -EIO;
562578

563579
sess_id = le64_to_cpu(req_hdr->SessionId);
580+
581+
/*
582+
* If request is not the first in Compound request,
583+
* Just validate session id in header with work->sess->id.
584+
*/
585+
if (work->next_smb2_rcv_hdr_off) {
586+
if (!work->sess) {
587+
pr_err("The first operation in the compound does not have sess\n");
588+
return -EINVAL;
589+
}
590+
if (work->sess->id != sess_id) {
591+
pr_err("session id(%llu) is different with the first operation(%lld)\n",
592+
sess_id, work->sess->id);
593+
return -EINVAL;
594+
}
595+
return 1;
596+
}
597+
564598
/* Check for validity of user session */
565599
work->sess = ksmbd_session_lookup_all(conn, sess_id);
566600
if (work->sess)
567601
return 1;
568602
ksmbd_debug(SMB, "Invalid user session, Uid %llu\n", sess_id);
569-
return -EINVAL;
603+
return -ENOENT;
570604
}
571605

572606
static void destroy_previous_session(struct ksmbd_conn *conn,
@@ -2249,7 +2283,7 @@ static int smb2_set_ea(struct smb2_ea_info *eabuf, unsigned int buf_len,
22492283
/* delete the EA only when it exits */
22502284
if (rc > 0) {
22512285
rc = ksmbd_vfs_remove_xattr(idmap,
2252-
path->dentry,
2286+
path,
22532287
attr_name);
22542288

22552289
if (rc < 0) {
@@ -2263,8 +2297,7 @@ static int smb2_set_ea(struct smb2_ea_info *eabuf, unsigned int buf_len,
22632297
/* if the EA doesn't exist, just do nothing. */
22642298
rc = 0;
22652299
} else {
2266-
rc = ksmbd_vfs_setxattr(idmap,
2267-
path->dentry, attr_name, value,
2300+
rc = ksmbd_vfs_setxattr(idmap, path, attr_name, value,
22682301
le16_to_cpu(eabuf->EaValueLength), 0);
22692302
if (rc < 0) {
22702303
ksmbd_debug(SMB,
@@ -2321,8 +2354,7 @@ static noinline int smb2_set_stream_name_xattr(const struct path *path,
23212354
return -EBADF;
23222355
}
23232356

2324-
rc = ksmbd_vfs_setxattr(idmap, path->dentry,
2325-
xattr_stream_name, NULL, 0, 0);
2357+
rc = ksmbd_vfs_setxattr(idmap, path, xattr_stream_name, NULL, 0, 0);
23262358
if (rc < 0)
23272359
pr_err("Failed to store XATTR stream name :%d\n", rc);
23282360
return 0;
@@ -2350,7 +2382,7 @@ static int smb2_remove_smb_xattrs(const struct path *path)
23502382
if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN) &&
23512383
!strncmp(&name[XATTR_USER_PREFIX_LEN], STREAM_PREFIX,
23522384
STREAM_PREFIX_LEN)) {
2353-
err = ksmbd_vfs_remove_xattr(idmap, path->dentry,
2385+
err = ksmbd_vfs_remove_xattr(idmap, path,
23542386
name);
23552387
if (err)
23562388
ksmbd_debug(SMB, "remove xattr failed : %s\n",
@@ -2397,8 +2429,7 @@ static void smb2_new_xattrs(struct ksmbd_tree_connect *tcon, const struct path *
23972429
da.flags = XATTR_DOSINFO_ATTRIB | XATTR_DOSINFO_CREATE_TIME |
23982430
XATTR_DOSINFO_ITIME;
23992431

2400-
rc = ksmbd_vfs_set_dos_attrib_xattr(mnt_idmap(path->mnt),
2401-
path->dentry, &da);
2432+
rc = ksmbd_vfs_set_dos_attrib_xattr(mnt_idmap(path->mnt), path, &da);
24022433
if (rc)
24032434
ksmbd_debug(SMB, "failed to store file attribute into xattr\n");
24042435
}
@@ -2972,7 +3003,7 @@ int smb2_open(struct ksmbd_work *work)
29723003
struct inode *inode = d_inode(path.dentry);
29733004

29743005
posix_acl_rc = ksmbd_vfs_inherit_posix_acl(idmap,
2975-
path.dentry,
3006+
&path,
29763007
d_inode(path.dentry->d_parent));
29773008
if (posix_acl_rc)
29783009
ksmbd_debug(SMB, "inherit posix acl failed : %d\n", posix_acl_rc);
@@ -2988,7 +3019,7 @@ int smb2_open(struct ksmbd_work *work)
29883019
if (rc) {
29893020
if (posix_acl_rc)
29903021
ksmbd_vfs_set_init_posix_acl(idmap,
2991-
path.dentry);
3022+
&path);
29923023

29933024
if (test_share_config_flag(work->tcon->share_conf,
29943025
KSMBD_SHARE_FLAG_ACL_XATTR)) {
@@ -3028,7 +3059,7 @@ int smb2_open(struct ksmbd_work *work)
30283059

30293060
rc = ksmbd_vfs_set_sd_xattr(conn,
30303061
idmap,
3031-
path.dentry,
3062+
&path,
30323063
pntsd,
30333064
pntsd_size);
30343065
kfree(pntsd);
@@ -5464,7 +5495,7 @@ static int smb2_rename(struct ksmbd_work *work,
54645495
goto out;
54655496

54665497
rc = ksmbd_vfs_setxattr(file_mnt_idmap(fp->filp),
5467-
fp->filp->f_path.dentry,
5498+
&fp->filp->f_path,
54685499
xattr_stream_name,
54695500
NULL, 0, 0);
54705501
if (rc < 0) {
@@ -5629,8 +5660,7 @@ static int set_file_basic_info(struct ksmbd_file *fp,
56295660
da.flags = XATTR_DOSINFO_ATTRIB | XATTR_DOSINFO_CREATE_TIME |
56305661
XATTR_DOSINFO_ITIME;
56315662

5632-
rc = ksmbd_vfs_set_dos_attrib_xattr(idmap,
5633-
filp->f_path.dentry, &da);
5663+
rc = ksmbd_vfs_set_dos_attrib_xattr(idmap, &filp->f_path, &da);
56345664
if (rc)
56355665
ksmbd_debug(SMB,
56365666
"failed to restore file attribute in EA\n");
@@ -7485,7 +7515,7 @@ static inline int fsctl_set_sparse(struct ksmbd_work *work, u64 id,
74857515

74867516
da.attr = le32_to_cpu(fp->f_ci->m_fattr);
74877517
ret = ksmbd_vfs_set_dos_attrib_xattr(idmap,
7488-
fp->filp->f_path.dentry, &da);
7518+
&fp->filp->f_path, &da);
74897519
if (ret)
74907520
fp->f_ci->m_fattr = old_fattr;
74917521
}

‎fs/smb/server/smbacl.c‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,8 +1162,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,
11621162
pntsd_size += sizeof(struct smb_acl) + nt_size;
11631163
}
11641164

1165-
ksmbd_vfs_set_sd_xattr(conn, idmap,
1166-
path->dentry, pntsd, pntsd_size);
1165+
ksmbd_vfs_set_sd_xattr(conn, idmap, path, pntsd, pntsd_size);
11671166
kfree(pntsd);
11681167
}
11691168

@@ -1383,7 +1382,7 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
13831382
newattrs.ia_valid |= ATTR_MODE;
13841383
newattrs.ia_mode = (inode->i_mode & ~0777) | (fattr.cf_mode & 0777);
13851384

1386-
ksmbd_vfs_remove_acl_xattrs(idmap, path->dentry);
1385+
ksmbd_vfs_remove_acl_xattrs(idmap, path);
13871386
/* Update posix acls */
13881387
if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && fattr.cf_dacls) {
13891388
rc = set_posix_acl(idmap, path->dentry,
@@ -1414,9 +1413,8 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
14141413

14151414
if (test_share_config_flag(tcon->share_conf, KSMBD_SHARE_FLAG_ACL_XATTR)) {
14161415
/* Update WinACL in xattr */
1417-
ksmbd_vfs_remove_sd_xattrs(idmap, path->dentry);
1418-
ksmbd_vfs_set_sd_xattr(conn, idmap,
1419-
path->dentry, pntsd, ntsd_len);
1416+
ksmbd_vfs_remove_sd_xattrs(idmap, path);
1417+
ksmbd_vfs_set_sd_xattr(conn, idmap, path, pntsd, ntsd_len);
14201418
}
14211419

14221420
out:

0 commit comments

Comments
 (0)