* [PATCH v2 1/5] ksmbd: fix memory leak in smb2_handle_negotiate
@ 2022-07-28 13:49 Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 2/5] ksmbd: fix use-after-free bug in smb2_tree_disconect Namjae Jeon
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Namjae Jeon @ 2022-07-28 13:49 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, hyc.lee, senozhatsky, gregkh, Namjae Jeon, stable,
zdi-disclosures
The allocated memory didn't free under an error
path in smb2_handle_negotiate().
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17815
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
---
v2:
- add missing fixes and stable tags.
fs/ksmbd/smb2pdu.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 1f4f2d5217a6..41ef076af072 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -1142,12 +1142,16 @@ int smb2_handle_negotiate(struct ksmbd_work *work)
status);
rsp->hdr.Status = status;
rc = -EINVAL;
+ kfree(conn->preauth_info);
+ conn->preauth_info = NULL;
goto err_out;
}
rc = init_smb3_11_server(conn);
if (rc < 0) {
rsp->hdr.Status = STATUS_INVALID_PARAMETER;
+ kfree(conn->preauth_info);
+ conn->preauth_info = NULL;
goto err_out;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/5] ksmbd: fix use-after-free bug in smb2_tree_disconect
2022-07-28 13:49 [PATCH v2 1/5] ksmbd: fix memory leak in smb2_handle_negotiate Namjae Jeon
@ 2022-07-28 13:49 ` Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 3/5] ksmbd: prevent out of bound read for SMB2_WRITE Namjae Jeon
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Namjae Jeon @ 2022-07-28 13:49 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, hyc.lee, senozhatsky, gregkh, Namjae Jeon, stable,
zdi-disclosures
smb2_tree_disconnect() freed the struct ksmbd_tree_connect,
but it left the dangling pointer. It can be accessed
again under compound requests.
This bug can lead an oops looking something link:
[ 1685.468014 ] BUG: KASAN: use-after-free in ksmbd_tree_conn_disconnect+0x131/0x160 [ksmbd]
[ 1685.468068 ] Read of size 4 at addr ffff888102172180 by task kworker/1:2/4807
...
[ 1685.468130 ] Call Trace:
[ 1685.468132 ] <TASK>
[ 1685.468135 ] dump_stack_lvl+0x49/0x5f
[ 1685.468141 ] print_report.cold+0x5e/0x5cf
[ 1685.468145 ] ? ksmbd_tree_conn_disconnect+0x131/0x160 [ksmbd]
[ 1685.468157 ] kasan_report+0xaa/0x120
[ 1685.468194 ] ? ksmbd_tree_conn_disconnect+0x131/0x160 [ksmbd]
[ 1685.468206 ] __asan_report_load4_noabort+0x14/0x20
[ 1685.468210 ] ksmbd_tree_conn_disconnect+0x131/0x160 [ksmbd]
[ 1685.468222 ] smb2_tree_disconnect+0x175/0x250 [ksmbd]
[ 1685.468235 ] handle_ksmbd_work+0x30e/0x1020 [ksmbd]
[ 1685.468247 ] process_one_work+0x778/0x11c0
[ 1685.468251 ] ? _raw_spin_lock_irq+0x8e/0xe0
[ 1685.468289 ] worker_thread+0x544/0x1180
[ 1685.468293 ] ? __cpuidle_text_end+0x4/0x4
[ 1685.468297 ] kthread+0x282/0x320
[ 1685.468301 ] ? process_one_work+0x11c0/0x11c0
[ 1685.468305 ] ? kthread_complete_and_exit+0x30/0x30
[ 1685.468309 ] ret_from_fork+0x1f/0x30
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17816
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
---
v2:
- add missing fixes and stable tags.
fs/ksmbd/smb2pdu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 41ef076af072..7ecb6d87ae3e 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -2057,6 +2057,7 @@ int smb2_tree_disconnect(struct ksmbd_work *work)
ksmbd_close_tree_conn_fds(work);
ksmbd_tree_conn_disconnect(sess, tcon);
+ work->tcon = NULL;
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/5] ksmbd: prevent out of bound read for SMB2_WRITE
2022-07-28 13:49 [PATCH v2 1/5] ksmbd: fix memory leak in smb2_handle_negotiate Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 2/5] ksmbd: fix use-after-free bug in smb2_tree_disconect Namjae Jeon
@ 2022-07-28 13:49 ` Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 4/5] ksmbd: prevent out of bound read for SMB2_TREE_CONNNECT Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 5/5] ksmbd: fix heap-based overflow in set_ntacl_dacl() Namjae Jeon
3 siblings, 0 replies; 5+ messages in thread
From: Namjae Jeon @ 2022-07-28 13:49 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, hyc.lee, senozhatsky, gregkh, Namjae Jeon, stable,
zdi-disclosures
From: Hyunchul Lee <hyc.lee@gmail.com>
OOB read memory can be written to a file,
if DataOffset is 0 and Length is too large
in SMB2_WRITE request of compound request.
To prevent this, when checking the length of
the data area of SMB2_WRITE in smb2_get_data_area_len(),
let the minimum of DataOffset be the size of
SMB2 header + the size of SMB2_WRITE header.
This bug can lead an oops looking something like:
[ 798.008715] BUG: KASAN: slab-out-of-bounds in copy_page_from_iter_atomic+0xd3d/0x14b0
[ 798.008724] Read of size 252 at addr ffff88800f863e90 by task kworker/0:2/2859
...
[ 798.008754] Call Trace:
[ 798.008756] <TASK>
[ 798.008759] dump_stack_lvl+0x49/0x5f
[ 798.008764] print_report.cold+0x5e/0x5cf
[ 798.008768] ? __filemap_get_folio+0x285/0x6d0
[ 798.008774] ? copy_page_from_iter_atomic+0xd3d/0x14b0
[ 798.008777] kasan_report+0xaa/0x120
[ 798.008781] ? copy_page_from_iter_atomic+0xd3d/0x14b0
[ 798.008784] kasan_check_range+0x100/0x1e0
[ 798.008788] memcpy+0x24/0x60
[ 798.008792] copy_page_from_iter_atomic+0xd3d/0x14b0
[ 798.008795] ? pagecache_get_page+0x53/0x160
[ 798.008799] ? iov_iter_get_pages_alloc+0x1590/0x1590
[ 798.008803] ? ext4_write_begin+0xfc0/0xfc0
[ 798.008807] ? current_time+0x72/0x210
[ 798.008811] generic_perform_write+0x2c8/0x530
[ 798.008816] ? filemap_fdatawrite_wbc+0x180/0x180
[ 798.008820] ? down_write+0xb4/0x120
[ 798.008824] ? down_write_killable+0x130/0x130
[ 798.008829] ext4_buffered_write_iter+0x137/0x2c0
[ 798.008833] ext4_file_write_iter+0x40b/0x1490
[ 798.008837] ? __fsnotify_parent+0x275/0xb20
[ 798.008842] ? __fsnotify_update_child_dentry_flags+0x2c0/0x2c0
[ 798.008846] ? ext4_buffered_write_iter+0x2c0/0x2c0
[ 798.008851] __kernel_write+0x3a1/0xa70
[ 798.008855] ? __x64_sys_preadv2+0x160/0x160
[ 798.008860] ? security_file_permission+0x4a/0xa0
[ 798.008865] kernel_write+0xbb/0x360
[ 798.008869] ksmbd_vfs_write+0x27e/0xb90 [ksmbd]
[ 798.008881] ? ksmbd_vfs_read+0x830/0x830 [ksmbd]
[ 798.008892] ? _raw_read_unlock+0x2a/0x50
[ 798.008896] smb2_write+0xb45/0x14e0 [ksmbd]
[ 798.008909] ? __kasan_check_write+0x14/0x20
[ 798.008912] ? _raw_spin_lock_bh+0xd0/0xe0
[ 798.008916] ? smb2_read+0x15e0/0x15e0 [ksmbd]
[ 798.008927] ? memcpy+0x4e/0x60
[ 798.008931] ? _raw_spin_unlock+0x19/0x30
[ 798.008934] ? ksmbd_smb2_check_message+0x16af/0x2350 [ksmbd]
[ 798.008946] ? _raw_spin_lock_bh+0xe0/0xe0
[ 798.008950] handle_ksmbd_work+0x30e/0x1020 [ksmbd]
[ 798.008962] process_one_work+0x778/0x11c0
[ 798.008966] ? _raw_spin_lock_irq+0x8e/0xe0
[ 798.008970] worker_thread+0x544/0x1180
[ 798.008973] ? __cpuidle_text_end+0x4/0x4
[ 798.008977] kthread+0x282/0x320
[ 798.008982] ? process_one_work+0x11c0/0x11c0
[ 798.008985] ? kthread_complete_and_exit+0x30/0x30
[ 798.008989] ret_from_fork+0x1f/0x30
[ 798.008995] </TASK>
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17817
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
---
v2:
- add missing fixes and stable tags.
fs/ksmbd/smb2misc.c | 7 +++++--
fs/ksmbd/smb2pdu.c | 7 ++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index f8f456377a51..aa1e663d9deb 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -136,8 +136,11 @@ static int smb2_get_data_area_len(unsigned int *off, unsigned int *len,
*len = le16_to_cpu(((struct smb2_read_req *)hdr)->ReadChannelInfoLength);
break;
case SMB2_WRITE:
- if (((struct smb2_write_req *)hdr)->DataOffset) {
- *off = le16_to_cpu(((struct smb2_write_req *)hdr)->DataOffset);
+ if (((struct smb2_write_req *)hdr)->DataOffset ||
+ ((struct smb2_write_req *)hdr)->Length) {
+ *off = max_t(unsigned int,
+ le16_to_cpu(((struct smb2_write_req *)hdr)->DataOffset),
+ offsetof(struct smb2_write_req, Buffer));
*len = le32_to_cpu(((struct smb2_write_req *)hdr)->Length);
break;
}
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 7ecb6d87ae3e..1bad4f729160 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -6514,14 +6514,11 @@ int smb2_write(struct ksmbd_work *work)
writethrough = true;
if (is_rdma_channel == false) {
- if ((u64)le16_to_cpu(req->DataOffset) + length >
- get_rfc1002_len(work->request_buf)) {
- pr_err("invalid write data offset %u, smb_len %u\n",
- le16_to_cpu(req->DataOffset),
- get_rfc1002_len(work->request_buf));
+ if (req->DataOffset < offsetof(struct smb2_write_req, Buffer)) {
err = -EINVAL;
goto out;
}
+
data_buf = (char *)(((char *)&req->hdr.ProtocolId) +
le16_to_cpu(req->DataOffset));
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 4/5] ksmbd: prevent out of bound read for SMB2_TREE_CONNNECT
2022-07-28 13:49 [PATCH v2 1/5] ksmbd: fix memory leak in smb2_handle_negotiate Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 2/5] ksmbd: fix use-after-free bug in smb2_tree_disconect Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 3/5] ksmbd: prevent out of bound read for SMB2_WRITE Namjae Jeon
@ 2022-07-28 13:49 ` Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 5/5] ksmbd: fix heap-based overflow in set_ntacl_dacl() Namjae Jeon
3 siblings, 0 replies; 5+ messages in thread
From: Namjae Jeon @ 2022-07-28 13:49 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, hyc.lee, senozhatsky, gregkh, Namjae Jeon, stable,
zdi-disclosures
From: Hyunchul Lee <hyc.lee@gmail.com>
if Status is not 0 and PathLength is long,
smb_strndup_from_utf16 could make out of bound
read in smb2_tree_connnect.
This bug can lead an oops looking something like:
[ 1553.882047] BUG: KASAN: slab-out-of-bounds in smb_strndup_from_utf16+0x469/0x4c0 [ksmbd]
[ 1553.882064] Read of size 2 at addr ffff88802c4eda04 by task kworker/0:2/42805
...
[ 1553.882095] Call Trace:
[ 1553.882098] <TASK>
[ 1553.882101] dump_stack_lvl+0x49/0x5f
[ 1553.882107] print_report.cold+0x5e/0x5cf
[ 1553.882112] ? smb_strndup_from_utf16+0x469/0x4c0 [ksmbd]
[ 1553.882122] kasan_report+0xaa/0x120
[ 1553.882128] ? smb_strndup_from_utf16+0x469/0x4c0 [ksmbd]
[ 1553.882139] __asan_report_load_n_noabort+0xf/0x20
[ 1553.882143] smb_strndup_from_utf16+0x469/0x4c0 [ksmbd]
[ 1553.882155] ? smb_strtoUTF16+0x3b0/0x3b0 [ksmbd]
[ 1553.882166] ? __kmalloc_node+0x185/0x430
[ 1553.882171] smb2_tree_connect+0x140/0xab0 [ksmbd]
[ 1553.882185] handle_ksmbd_work+0x30e/0x1020 [ksmbd]
[ 1553.882197] process_one_work+0x778/0x11c0
[ 1553.882201] ? _raw_spin_lock_irq+0x8e/0xe0
[ 1553.882206] worker_thread+0x544/0x1180
[ 1553.882209] ? __cpuidle_text_end+0x4/0x4
[ 1553.882214] kthread+0x282/0x320
[ 1553.882218] ? process_one_work+0x11c0/0x11c0
[ 1553.882221] ? kthread_complete_and_exit+0x30/0x30
[ 1553.882225] ret_from_fork+0x1f/0x30
[ 1553.882231] </TASK>
There is no need to check error request validation in server.
This check allow invalid requests not to validate message.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17818
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
---
v2:
- add missing fixes and stable tags.
fs/ksmbd/smb2misc.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index aa1e663d9deb..6e25ace36568 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -90,11 +90,6 @@ static int smb2_get_data_area_len(unsigned int *off, unsigned int *len,
*off = 0;
*len = 0;
- /* error reqeusts do not have data area */
- if (hdr->Status && hdr->Status != STATUS_MORE_PROCESSING_REQUIRED &&
- (((struct smb2_err_rsp *)hdr)->StructureSize) == SMB2_ERROR_STRUCTURE_SIZE2_LE)
- return ret;
-
/*
* Following commands have data areas so we have to get the location
* of the data buffer offset and data buffer length for the particular
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 5/5] ksmbd: fix heap-based overflow in set_ntacl_dacl()
2022-07-28 13:49 [PATCH v2 1/5] ksmbd: fix memory leak in smb2_handle_negotiate Namjae Jeon
` (2 preceding siblings ...)
2022-07-28 13:49 ` [PATCH v2 4/5] ksmbd: prevent out of bound read for SMB2_TREE_CONNNECT Namjae Jeon
@ 2022-07-28 13:49 ` Namjae Jeon
3 siblings, 0 replies; 5+ messages in thread
From: Namjae Jeon @ 2022-07-28 13:49 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, hyc.lee, senozhatsky, gregkh, Namjae Jeon, stable,
zdi-disclosures
The testcase use SMB2_SET_INFO_HE command to set a malformed file attribute
under the label `security.NTACL`. SMB2_QUERY_INFO_HE command in testcase
trigger the following overflow.
[ 4712.003781] ==================================================================
[ 4712.003790] BUG: KASAN: slab-out-of-bounds in build_sec_desc+0x842/0x1dd0 [ksmbd]
[ 4712.003807] Write of size 1060 at addr ffff88801e34c068 by task kworker/0:0/4190
[ 4712.003813] CPU: 0 PID: 4190 Comm: kworker/0:0 Not tainted 5.19.0-rc5 #1
[ 4712.003850] Workqueue: ksmbd-io handle_ksmbd_work [ksmbd]
[ 4712.003867] Call Trace:
[ 4712.003870] <TASK>
[ 4712.003873] dump_stack_lvl+0x49/0x5f
[ 4712.003935] print_report.cold+0x5e/0x5cf
[ 4712.003972] ? ksmbd_vfs_get_sd_xattr+0x16d/0x500 [ksmbd]
[ 4712.003984] ? cmp_map_id+0x200/0x200
[ 4712.003988] ? build_sec_desc+0x842/0x1dd0 [ksmbd]
[ 4712.004000] kasan_report+0xaa/0x120
[ 4712.004045] ? build_sec_desc+0x842/0x1dd0 [ksmbd]
[ 4712.004056] kasan_check_range+0x100/0x1e0
[ 4712.004060] memcpy+0x3c/0x60
[ 4712.004064] build_sec_desc+0x842/0x1dd0 [ksmbd]
[ 4712.004076] ? parse_sec_desc+0x580/0x580 [ksmbd]
[ 4712.004088] ? ksmbd_acls_fattr+0x281/0x410 [ksmbd]
[ 4712.004099] smb2_query_info+0xa8f/0x6110 [ksmbd]
[ 4712.004111] ? psi_group_change+0x856/0xd70
[ 4712.004148] ? update_load_avg+0x1c3/0x1af0
[ 4712.004152] ? asym_cpu_capacity_scan+0x5d0/0x5d0
[ 4712.004157] ? xas_load+0x23/0x300
[ 4712.004162] ? smb2_query_dir+0x1530/0x1530 [ksmbd]
[ 4712.004173] ? _raw_spin_lock_bh+0xe0/0xe0
[ 4712.004179] handle_ksmbd_work+0x30e/0x1020 [ksmbd]
[ 4712.004192] process_one_work+0x778/0x11c0
[ 4712.004227] ? _raw_spin_lock_irq+0x8e/0xe0
[ 4712.004231] worker_thread+0x544/0x1180
[ 4712.004234] ? __cpuidle_text_end+0x4/0x4
[ 4712.004239] kthread+0x282/0x320
[ 4712.004243] ? process_one_work+0x11c0/0x11c0
[ 4712.004246] ? kthread_complete_and_exit+0x30/0x30
[ 4712.004282] ret_from_fork+0x1f/0x30
This patch add the buffer validation for security descriptor that is
stored by malformed SMB2_SET_INFO_HE command. and allocate large
response buffer about SMB2_O_INFO_SECURITY file info class.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17771
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
v2:
- add missing fixes and stable tags.
fs/ksmbd/smb2pdu.c | 37 ++++++++-----
fs/ksmbd/smbacl.c | 130 ++++++++++++++++++++++++++++++---------------
fs/ksmbd/smbacl.h | 2 +-
fs/ksmbd/vfs.c | 5 ++
4 files changed, 118 insertions(+), 56 deletions(-)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 1bad4f729160..71978ccbd4b3 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -535,9 +535,10 @@ int smb2_allocate_rsp_buf(struct ksmbd_work *work)
struct smb2_query_info_req *req;
req = smb2_get_msg(work->request_buf);
- if (req->InfoType == SMB2_O_INFO_FILE &&
- (req->FileInfoClass == FILE_FULL_EA_INFORMATION ||
- req->FileInfoClass == FILE_ALL_INFORMATION))
+ if ((req->InfoType == SMB2_O_INFO_FILE &&
+ (req->FileInfoClass == FILE_FULL_EA_INFORMATION ||
+ req->FileInfoClass == FILE_ALL_INFORMATION)) ||
+ req->InfoType == SMB2_O_INFO_SECURITY)
sz = large_sz;
}
@@ -2988,7 +2989,7 @@ int smb2_open(struct ksmbd_work *work)
goto err_out;
rc = build_sec_desc(user_ns,
- pntsd, NULL,
+ pntsd, NULL, 0,
OWNER_SECINFO |
GROUP_SECINFO |
DACL_SECINFO,
@@ -3833,6 +3834,15 @@ static int verify_info_level(int info_level)
return 0;
}
+static int smb2_resp_buf_len(struct ksmbd_work *work, unsigned short hdr2_len)
+{
+ int free_len;
+
+ free_len = (int)(work->response_sz -
+ (get_rfc1002_len(work->response_buf) + 4)) - hdr2_len;
+ return free_len;
+}
+
static int smb2_calc_max_out_buf_len(struct ksmbd_work *work,
unsigned short hdr2_len,
unsigned int out_buf_len)
@@ -3842,9 +3852,7 @@ static int smb2_calc_max_out_buf_len(struct ksmbd_work *work,
if (out_buf_len > work->conn->vals->max_trans_size)
return -EINVAL;
- free_len = (int)(work->response_sz -
- (get_rfc1002_len(work->response_buf) + 4)) -
- hdr2_len;
+ free_len = smb2_resp_buf_len(work, hdr2_len);
if (free_len < 0)
return -EINVAL;
@@ -5110,7 +5118,7 @@ static int smb2_get_info_sec(struct ksmbd_work *work,
__u32 secdesclen;
unsigned int id = KSMBD_NO_FID, pid = KSMBD_NO_FID;
int addition_info = le32_to_cpu(req->AdditionalInformation);
- int rc;
+ int rc = 0, ppntsd_size = 0;
if (addition_info & ~(OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO |
PROTECTED_DACL_SECINFO |
@@ -5156,11 +5164,14 @@ static int smb2_get_info_sec(struct ksmbd_work *work,
if (test_share_config_flag(work->tcon->share_conf,
KSMBD_SHARE_FLAG_ACL_XATTR))
- ksmbd_vfs_get_sd_xattr(work->conn, user_ns,
- fp->filp->f_path.dentry, &ppntsd);
-
- rc = build_sec_desc(user_ns, pntsd, ppntsd, addition_info,
- &secdesclen, &fattr);
+ ppntsd_size = ksmbd_vfs_get_sd_xattr(work->conn, user_ns,
+ fp->filp->f_path.dentry,
+ &ppntsd);
+
+ /* Check if sd buffer size exceeds response buffer size */
+ if (smb2_resp_buf_len(work, 8) > ppntsd_size)
+ rc = build_sec_desc(user_ns, pntsd, ppntsd, ppntsd_size,
+ addition_info, &secdesclen, &fattr);
posix_acl_release(fattr.cf_acls);
posix_acl_release(fattr.cf_dacls);
kfree(ppntsd);
diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c
index 38f23bf981ac..d58816d274f5 100644
--- a/fs/ksmbd/smbacl.c
+++ b/fs/ksmbd/smbacl.c
@@ -690,6 +690,7 @@ static void set_posix_acl_entries_dacl(struct user_namespace *user_ns,
static void set_ntacl_dacl(struct user_namespace *user_ns,
struct smb_acl *pndacl,
struct smb_acl *nt_dacl,
+ unsigned int aces_size,
const struct smb_sid *pownersid,
const struct smb_sid *pgrpsid,
struct smb_fattr *fattr)
@@ -703,9 +704,19 @@ static void set_ntacl_dacl(struct user_namespace *user_ns,
if (nt_num_aces) {
ntace = (struct smb_ace *)((char *)nt_dacl + sizeof(struct smb_acl));
for (i = 0; i < nt_num_aces; i++) {
- memcpy((char *)pndace + size, ntace, le16_to_cpu(ntace->size));
- size += le16_to_cpu(ntace->size);
- ntace = (struct smb_ace *)((char *)ntace + le16_to_cpu(ntace->size));
+ unsigned short nt_ace_size;
+
+ if (offsetof(struct smb_ace, access_req) > aces_size)
+ break;
+
+ nt_ace_size = le16_to_cpu(ntace->size);
+ if (nt_ace_size > aces_size)
+ break;
+
+ memcpy((char *)pndace + size, ntace, nt_ace_size);
+ size += nt_ace_size;
+ aces_size -= nt_ace_size;
+ ntace = (struct smb_ace *)((char *)ntace + nt_ace_size);
num_aces++;
}
}
@@ -878,7 +889,7 @@ int parse_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd,
/* Convert permission bits from mode to equivalent CIFS ACL */
int build_sec_desc(struct user_namespace *user_ns,
struct smb_ntsd *pntsd, struct smb_ntsd *ppntsd,
- int addition_info, __u32 *secdesclen,
+ int ppntsd_size, int addition_info, __u32 *secdesclen,
struct smb_fattr *fattr)
{
int rc = 0;
@@ -938,15 +949,25 @@ int build_sec_desc(struct user_namespace *user_ns,
if (!ppntsd) {
set_mode_dacl(user_ns, dacl_ptr, fattr);
- } else if (!ppntsd->dacloffset) {
- goto out;
} else {
struct smb_acl *ppdacl_ptr;
+ unsigned int dacl_offset = le32_to_cpu(ppntsd->dacloffset);
+ int ppdacl_size, ntacl_size = ppntsd_size - dacl_offset;
+
+ if (!dacl_offset ||
+ (dacl_offset + sizeof(struct smb_acl) > ppntsd_size))
+ goto out;
+
+ ppdacl_ptr = (struct smb_acl *)((char *)ppntsd + dacl_offset);
+ ppdacl_size = le16_to_cpu(ppdacl_ptr->size);
+ if (ppdacl_size > ntacl_size ||
+ ppdacl_size < sizeof(struct smb_acl))
+ goto out;
- ppdacl_ptr = (struct smb_acl *)((char *)ppntsd +
- le32_to_cpu(ppntsd->dacloffset));
set_ntacl_dacl(user_ns, dacl_ptr, ppdacl_ptr,
- nowner_sid_ptr, ngroup_sid_ptr, fattr);
+ ntacl_size - sizeof(struct smb_acl),
+ nowner_sid_ptr, ngroup_sid_ptr,
+ fattr);
}
pntsd->dacloffset = cpu_to_le32(offset);
offset += le16_to_cpu(dacl_ptr->size);
@@ -980,24 +1001,31 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,
struct smb_sid owner_sid, group_sid;
struct dentry *parent = path->dentry->d_parent;
struct user_namespace *user_ns = mnt_user_ns(path->mnt);
- int inherited_flags = 0, flags = 0, i, ace_cnt = 0, nt_size = 0;
- int rc = 0, num_aces, dacloffset, pntsd_type, acl_len;
+ int inherited_flags = 0, flags = 0, i, ace_cnt = 0, nt_size = 0, pdacl_size;
+ int rc = 0, num_aces, dacloffset, pntsd_type, pntsd_size, acl_len, aces_size;
char *aces_base;
bool is_dir = S_ISDIR(d_inode(path->dentry)->i_mode);
- acl_len = ksmbd_vfs_get_sd_xattr(conn, user_ns,
- parent, &parent_pntsd);
- if (acl_len <= 0)
+ pntsd_size = ksmbd_vfs_get_sd_xattr(conn, user_ns,
+ parent, &parent_pntsd);
+ if (pntsd_size <= 0)
return -ENOENT;
dacloffset = le32_to_cpu(parent_pntsd->dacloffset);
- if (!dacloffset) {
+ if (!dacloffset || (dacloffset + sizeof(struct smb_acl) > pntsd_size)) {
rc = -EINVAL;
goto free_parent_pntsd;
}
parent_pdacl = (struct smb_acl *)((char *)parent_pntsd + dacloffset);
+ acl_len = pntsd_size - dacloffset;
num_aces = le32_to_cpu(parent_pdacl->num_aces);
pntsd_type = le16_to_cpu(parent_pntsd->type);
+ pdacl_size = le16_to_cpu(parent_pdacl->size);
+
+ if (pdacl_size > acl_len || pdacl_size < sizeof(struct smb_acl)) {
+ rc = -EINVAL;
+ goto free_parent_pntsd;
+ }
aces_base = kmalloc(sizeof(struct smb_ace) * num_aces * 2, GFP_KERNEL);
if (!aces_base) {
@@ -1008,11 +1036,23 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,
aces = (struct smb_ace *)aces_base;
parent_aces = (struct smb_ace *)((char *)parent_pdacl +
sizeof(struct smb_acl));
+ aces_size = acl_len - sizeof(struct smb_acl);
if (pntsd_type & DACL_AUTO_INHERITED)
inherited_flags = INHERITED_ACE;
for (i = 0; i < num_aces; i++) {
+ int pace_size;
+
+ if (offsetof(struct smb_ace, access_req) > aces_size)
+ break;
+
+ pace_size = le16_to_cpu(parent_aces->size);
+ if (pace_size > aces_size)
+ break;
+
+ aces_size -= pace_size;
+
flags = parent_aces->flags;
if (!smb_inherit_flags(flags, is_dir))
goto pass;
@@ -1057,8 +1097,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,
aces = (struct smb_ace *)((char *)aces + le16_to_cpu(aces->size));
ace_cnt++;
pass:
- parent_aces =
- (struct smb_ace *)((char *)parent_aces + le16_to_cpu(parent_aces->size));
+ parent_aces = (struct smb_ace *)((char *)parent_aces + pace_size);
}
if (nt_size > 0) {
@@ -1153,7 +1192,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path,
struct smb_ntsd *pntsd = NULL;
struct smb_acl *pdacl;
struct posix_acl *posix_acls;
- int rc = 0, acl_size;
+ int rc = 0, pntsd_size, acl_size, aces_size, pdacl_size, dacl_offset;
struct smb_sid sid;
int granted = le32_to_cpu(*pdaccess & ~FILE_MAXIMAL_ACCESS_LE);
struct smb_ace *ace;
@@ -1162,37 +1201,33 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path,
struct smb_ace *others_ace = NULL;
struct posix_acl_entry *pa_entry;
unsigned int sid_type = SIDOWNER;
- char *end_of_acl;
+ unsigned short ace_size;
ksmbd_debug(SMB, "check permission using windows acl\n");
- acl_size = ksmbd_vfs_get_sd_xattr(conn, user_ns,
- path->dentry, &pntsd);
- if (acl_size <= 0 || !pntsd || !pntsd->dacloffset) {
- kfree(pntsd);
- return 0;
- }
+ pntsd_size = ksmbd_vfs_get_sd_xattr(conn, user_ns,
+ path->dentry, &pntsd);
+ if (pntsd_size <= 0)
+ goto err_out;
+
+ dacl_offset = le32_to_cpu(pntsd->dacloffset);
+ if (!pntsd || !pntsd->dacloffset ||
+ (dacl_offset + sizeof(struct smb_acl) > pntsd_size))
+ goto err_out;
pdacl = (struct smb_acl *)((char *)pntsd + le32_to_cpu(pntsd->dacloffset));
- end_of_acl = ((char *)pntsd) + acl_size;
- if (end_of_acl <= (char *)pdacl) {
- kfree(pntsd);
- return 0;
- }
+ acl_size = pntsd_size - dacl_offset;
+ pdacl_size = le16_to_cpu(pdacl->size);
- if (end_of_acl < (char *)pdacl + le16_to_cpu(pdacl->size) ||
- le16_to_cpu(pdacl->size) < sizeof(struct smb_acl)) {
- kfree(pntsd);
- return 0;
- }
+ if (pdacl_size > acl_size || pdacl_size < sizeof(struct smb_acl))
+ goto err_out;
if (!pdacl->num_aces) {
- if (!(le16_to_cpu(pdacl->size) - sizeof(struct smb_acl)) &&
+ if (!(pdacl_size - sizeof(struct smb_acl)) &&
*pdaccess & ~(FILE_READ_CONTROL_LE | FILE_WRITE_DAC_LE)) {
rc = -EACCES;
goto err_out;
}
- kfree(pntsd);
- return 0;
+ goto err_out;
}
if (*pdaccess & FILE_MAXIMAL_ACCESS_LE) {
@@ -1200,11 +1235,16 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path,
DELETE;
ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
+ aces_size = acl_size - sizeof(struct smb_acl);
for (i = 0; i < le32_to_cpu(pdacl->num_aces); i++) {
+ if (offsetof(struct smb_ace, access_req) > aces_size)
+ goto err_out;
+ ace_size = le16_to_cpu(ace->size);
+ if (ace_size > aces_size)
+ break;
+ aces_size -= ace_size;
granted |= le32_to_cpu(ace->access_req);
ace = (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size));
- if (end_of_acl < (char *)ace)
- goto err_out;
}
if (!pdacl->num_aces)
@@ -1216,7 +1256,15 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path,
id_to_sid(uid, sid_type, &sid);
ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
+ aces_size = acl_size - sizeof(struct smb_acl);
for (i = 0; i < le32_to_cpu(pdacl->num_aces); i++) {
+ if (offsetof(struct smb_ace, access_req) > aces_size)
+ goto err_out;
+ ace_size = le16_to_cpu(ace->size);
+ if (ace_size > aces_size)
+ break;
+ aces_size -= ace_size;
+
if (!compare_sids(&sid, &ace->sid) ||
!compare_sids(&sid_unix_NFS_mode, &ace->sid)) {
found = 1;
@@ -1226,8 +1274,6 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path,
others_ace = ace;
ace = (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size));
- if (end_of_acl < (char *)ace)
- goto err_out;
}
if (*pdaccess & FILE_MAXIMAL_ACCESS_LE && found) {
diff --git a/fs/ksmbd/smbacl.h b/fs/ksmbd/smbacl.h
index 811af3309429..fcb2c83f2992 100644
--- a/fs/ksmbd/smbacl.h
+++ b/fs/ksmbd/smbacl.h
@@ -193,7 +193,7 @@ struct posix_acl_state {
int parse_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd,
int acl_len, struct smb_fattr *fattr);
int build_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd,
- struct smb_ntsd *ppntsd, int addition_info,
+ struct smb_ntsd *ppntsd, int ppntsd_size, int addition_info,
__u32 *secdesclen, struct smb_fattr *fattr);
int init_acl_state(struct posix_acl_state *state, int cnt);
void free_acl_state(struct posix_acl_state *state);
diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c
index 4f75b1436eab..7af7e0853f06 100644
--- a/fs/ksmbd/vfs.c
+++ b/fs/ksmbd/vfs.c
@@ -1539,6 +1539,11 @@ int ksmbd_vfs_get_sd_xattr(struct ksmbd_conn *conn,
}
*pntsd = acl.sd_buf;
+ if (acl.sd_size < sizeof(struct smb_ntsd)) {
+ pr_err("sd size is invalid\n");
+ goto out_free;
+ }
+
(*pntsd)->osidoffset = cpu_to_le32(le32_to_cpu((*pntsd)->osidoffset) -
NDR_NTSD_OFFSETOF);
(*pntsd)->gsidoffset = cpu_to_le32(le32_to_cpu((*pntsd)->gsidoffset) -
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-28 13:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-28 13:49 [PATCH v2 1/5] ksmbd: fix memory leak in smb2_handle_negotiate Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 2/5] ksmbd: fix use-after-free bug in smb2_tree_disconect Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 3/5] ksmbd: prevent out of bound read for SMB2_WRITE Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 4/5] ksmbd: prevent out of bound read for SMB2_TREE_CONNNECT Namjae Jeon
2022-07-28 13:49 ` [PATCH v2 5/5] ksmbd: fix heap-based overflow in set_ntacl_dacl() Namjae Jeon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.