From: Namjae Jeon linkinjeon@kernel.org
mainline inclusion from mainline-5.15-rc4 commit 9496e268e3af78a92778bf635488a8ec2dca8996 category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/I60T7G CVE: NA
Reference: https://git.kernel.org/torvalds/linux/c/9496e268e3af
-------------------------------
Add buffer validation in smb2_set_info, and remove unused variable in set_file_basic_info. and smb2_set_info infolevel functions take structure pointer argument.
Cc: Tom Talpey tom@talpey.com Cc: Ronnie Sahlberg ronniesahlberg@gmail.com Cc: Ralph Böhme slow@samba.org Cc: Sergey Senozhatsky senozhatsky@chromium.org Acked-by: Hyunchul Lee hyc.lee@gmail.com Reviewed-by: Ralph Boehme slow@samba.org Signed-off-by: Namjae Jeon linkinjeon@kernel.org Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Jason Yan yanaijie@huawei.com Signed-off-by: Zhong Jinghua zhongjinghua@huawei.com --- fs/ksmbd/smb2pdu.c | 149 ++++++++++++++++++++++++++++++++------------- 1 file changed, 107 insertions(+), 42 deletions(-)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index f96aff7f54f4..7ce7fa898a42 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -2102,15 +2102,21 @@ static noinline int create_smb2_pipe(struct ksmbd_work *work) * smb2_set_ea() - handler for setting extended attributes using set * info command * @eabuf: set info command buffer + * @buf_len: set info command buffer length * @path: dentry path for get ea * * Return: 0 on success, otherwise error */ -static int smb2_set_ea(struct smb2_ea_info *eabuf, struct path *path) +static int smb2_set_ea(struct smb2_ea_info *eabuf, unsigned int buf_len, + struct path *path) { char *attr_name = NULL, *value; int rc = 0; - int next = 0; + unsigned int next = 0; + + if (buf_len < sizeof(struct smb2_ea_info) + eabuf->EaNameLength + + le16_to_cpu(eabuf->EaValueLength)) + return -EINVAL;
attr_name = kmalloc(XATTR_NAME_MAX + 1, GFP_KERNEL); if (!attr_name) @@ -2172,7 +2178,13 @@ static int smb2_set_ea(struct smb2_ea_info *eabuf, struct path *path)
next: next = le32_to_cpu(eabuf->NextEntryOffset); + if (next == 0 || buf_len < next) + break; + buf_len -= next; eabuf = (struct smb2_ea_info *)((char *)eabuf + next); + if (next < (u32)eabuf->EaNameLength + le16_to_cpu(eabuf->EaValueLength)) + break; + } while (next != 0);
kfree(attr_name); @@ -2738,7 +2750,15 @@ int smb2_open(struct ksmbd_work *work)
created = true; if (ea_buf) { - rc = smb2_set_ea(&ea_buf->ea, &path); + if (le32_to_cpu(ea_buf->ccontext.DataLength) < + sizeof(struct smb2_ea_info)) { + rc = -EINVAL; + goto err_out; + } + + rc = smb2_set_ea(&ea_buf->ea, + le32_to_cpu(ea_buf->ccontext.DataLength), + &path); if (rc == -EOPNOTSUPP) rc = 0; else if (rc) @@ -5296,7 +5316,7 @@ static int smb2_rename(struct ksmbd_work *work, struct ksmbd_file *fp, static int smb2_create_link(struct ksmbd_work *work, struct ksmbd_share_config *share, struct smb2_file_link_info *file_info, - struct file *filp, + unsigned int buf_len, struct file *filp, struct nls_table *local_nls) { char *link_name = NULL, *target_name = NULL, *pathname = NULL; @@ -5304,6 +5324,10 @@ static int smb2_create_link(struct ksmbd_work *work, bool file_present = true; int rc;
+ if (buf_len < (u64)sizeof(struct smb2_file_link_info) + + le32_to_cpu(file_info->FileNameLength)) + return -EINVAL; + ksmbd_debug(SMB, "setting FILE_LINK_INFORMATION\n"); pathname = kmalloc(PATH_MAX, GFP_KERNEL); if (!pathname) @@ -5363,10 +5387,10 @@ static int smb2_create_link(struct ksmbd_work *work, return rc; }
-static int set_file_basic_info(struct ksmbd_file *fp, char *buf, +static int set_file_basic_info(struct ksmbd_file *fp, + struct smb2_file_basic_info *file_info, struct ksmbd_share_config *share) { - struct smb2_file_basic_info *file_info; struct iattr attrs; struct timespec64 ctime; struct file *filp; @@ -5376,7 +5400,6 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf, if (!(fp->daccess & FILE_WRITE_ATTRIBUTES_LE)) return -EACCES;
- file_info = (struct smb2_file_basic_info *)buf; attrs.ia_valid = 0; filp = fp->filp; inode = file_inode(filp); @@ -5451,7 +5474,8 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf, }
static int set_file_allocation_info(struct ksmbd_work *work, - struct ksmbd_file *fp, char *buf) + struct ksmbd_file *fp, + struct smb2_file_alloc_info *file_alloc_info) { /* * TODO : It's working fine only when store dos attributes @@ -5459,7 +5483,6 @@ static int set_file_allocation_info(struct ksmbd_work *work, * properly with any smb.conf option */
- struct smb2_file_alloc_info *file_alloc_info; loff_t alloc_blks; struct inode *inode; int rc; @@ -5467,7 +5490,6 @@ static int set_file_allocation_info(struct ksmbd_work *work, if (!(fp->daccess & FILE_WRITE_DATA_LE)) return -EACCES;
- file_alloc_info = (struct smb2_file_alloc_info *)buf; alloc_blks = (le64_to_cpu(file_alloc_info->AllocationSize) + 511) >> 9; inode = file_inode(fp->filp);
@@ -5503,9 +5525,8 @@ static int set_file_allocation_info(struct ksmbd_work *work, }
static int set_end_of_file_info(struct ksmbd_work *work, struct ksmbd_file *fp, - char *buf) + struct smb2_file_eof_info *file_eof_info) { - struct smb2_file_eof_info *file_eof_info; loff_t newsize; struct inode *inode; int rc; @@ -5513,7 +5534,6 @@ static int set_end_of_file_info(struct ksmbd_work *work, struct ksmbd_file *fp, if (!(fp->daccess & FILE_WRITE_DATA_LE)) return -EACCES;
- file_eof_info = (struct smb2_file_eof_info *)buf; newsize = le64_to_cpu(file_eof_info->EndOfFile); inode = file_inode(fp->filp);
@@ -5540,7 +5560,8 @@ static int set_end_of_file_info(struct ksmbd_work *work, struct ksmbd_file *fp, }
static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp, - char *buf) + struct smb2_file_rename_info *rename_info, + unsigned int buf_len) { struct ksmbd_file *parent_fp; struct dentry *parent; @@ -5552,6 +5573,10 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp, return -EACCES; }
+ if (buf_len < (u64)sizeof(struct smb2_file_rename_info) + + le32_to_cpu(rename_info->FileNameLength)) + return -EINVAL; + if (ksmbd_stream_fd(fp)) goto next;
@@ -5573,14 +5598,13 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp, } } next: - return smb2_rename(work, fp, - (struct smb2_file_rename_info *)buf, + return smb2_rename(work, fp, rename_info, work->sess->conn->local_nls); }
-static int set_file_disposition_info(struct ksmbd_file *fp, char *buf) +static int set_file_disposition_info(struct ksmbd_file *fp, + struct smb2_file_disposition_info *file_info) { - struct smb2_file_disposition_info *file_info; struct inode *inode;
if (!(fp->daccess & FILE_DELETE_LE)) { @@ -5589,7 +5613,6 @@ static int set_file_disposition_info(struct ksmbd_file *fp, char *buf) }
inode = file_inode(fp->filp); - file_info = (struct smb2_file_disposition_info *)buf; if (file_info->DeletePending) { if (S_ISDIR(inode->i_mode) && ksmbd_vfs_empty_dir(fp) == -ENOTEMPTY) @@ -5601,15 +5624,14 @@ static int set_file_disposition_info(struct ksmbd_file *fp, char *buf) return 0; }
-static int set_file_position_info(struct ksmbd_file *fp, char *buf) +static int set_file_position_info(struct ksmbd_file *fp, + struct smb2_file_pos_info *file_info) { - struct smb2_file_pos_info *file_info; loff_t current_byte_offset; unsigned long sector_size; struct inode *inode;
inode = file_inode(fp->filp); - file_info = (struct smb2_file_pos_info *)buf; current_byte_offset = le64_to_cpu(file_info->CurrentByteOffset); sector_size = inode->i_sb->s_blocksize;
@@ -5625,12 +5647,11 @@ static int set_file_position_info(struct ksmbd_file *fp, char *buf) return 0; }
-static int set_file_mode_info(struct ksmbd_file *fp, char *buf) +static int set_file_mode_info(struct ksmbd_file *fp, + struct smb2_file_mode_info *file_info) { - struct smb2_file_mode_info *file_info; __le32 mode;
- file_info = (struct smb2_file_mode_info *)buf; mode = file_info->Mode;
if ((mode & ~FILE_MODE_INFO_MASK) || @@ -5660,40 +5681,74 @@ static int set_file_mode_info(struct ksmbd_file *fp, char *buf) * TODO: need to implement an error handling for STATUS_INFO_LENGTH_MISMATCH */ static int smb2_set_info_file(struct ksmbd_work *work, struct ksmbd_file *fp, - int info_class, char *buf, + struct smb2_set_info_req *req, struct ksmbd_share_config *share) { - switch (info_class) { + unsigned int buf_len = le32_to_cpu(req->BufferLength); + + switch (req->FileInfoClass) { case FILE_BASIC_INFORMATION: - return set_file_basic_info(fp, buf, share); + { + if (buf_len < sizeof(struct smb2_file_basic_info)) + return -EINVAL;
+ return set_file_basic_info(fp, (struct smb2_file_basic_info *)req->Buffer, share); + } case FILE_ALLOCATION_INFORMATION: - return set_file_allocation_info(work, fp, buf); + { + if (buf_len < sizeof(struct smb2_file_alloc_info)) + return -EINVAL;
+ return set_file_allocation_info(work, fp, + (struct smb2_file_alloc_info *)req->Buffer); + } case FILE_END_OF_FILE_INFORMATION: - return set_end_of_file_info(work, fp, buf); + { + if (buf_len < sizeof(struct smb2_file_eof_info)) + return -EINVAL;
+ return set_end_of_file_info(work, fp, + (struct smb2_file_eof_info *)req->Buffer); + } case FILE_RENAME_INFORMATION: + { if (!test_tree_conn_flag(work->tcon, KSMBD_TREE_CONN_FLAG_WRITABLE)) { ksmbd_debug(SMB, "User does not have write permission\n"); return -EACCES; } - return set_rename_info(work, fp, buf);
+ if (buf_len < sizeof(struct smb2_file_rename_info)) + return -EINVAL; + + return set_rename_info(work, fp, + (struct smb2_file_rename_info *)req->Buffer, + buf_len); + } case FILE_LINK_INFORMATION: + { + if (buf_len < sizeof(struct smb2_file_link_info)) + return -EINVAL; + return smb2_create_link(work, work->tcon->share_conf, - (struct smb2_file_link_info *)buf, fp->filp, + (struct smb2_file_link_info *)req->Buffer, + buf_len, fp->filp, work->sess->conn->local_nls); - + } case FILE_DISPOSITION_INFORMATION: + { if (!test_tree_conn_flag(work->tcon, KSMBD_TREE_CONN_FLAG_WRITABLE)) { ksmbd_debug(SMB, "User does not have write permission\n"); return -EACCES; } - return set_file_disposition_info(fp, buf);
+ if (buf_len < sizeof(struct smb2_file_disposition_info)) + return -EINVAL; + + return set_file_disposition_info(fp, + (struct smb2_file_disposition_info *)req->Buffer); + } case FILE_FULL_EA_INFORMATION: { if (!(fp->daccess & FILE_WRITE_EA_LE)) { @@ -5702,18 +5757,29 @@ static int smb2_set_info_file(struct ksmbd_work *work, struct ksmbd_file *fp, return -EACCES; }
- return smb2_set_ea((struct smb2_ea_info *)buf, - &fp->filp->f_path); - } + if (buf_len < sizeof(struct smb2_ea_info)) + return -EINVAL;
+ return smb2_set_ea((struct smb2_ea_info *)req->Buffer, + buf_len, &fp->filp->f_path); + } case FILE_POSITION_INFORMATION: - return set_file_position_info(fp, buf); + { + if (buf_len < sizeof(struct smb2_file_pos_info)) + return -EINVAL;
+ return set_file_position_info(fp, (struct smb2_file_pos_info *)req->Buffer); + } case FILE_MODE_INFORMATION: - return set_file_mode_info(fp, buf); + { + if (buf_len < sizeof(struct smb2_file_mode_info)) + return -EINVAL; + + return set_file_mode_info(fp, (struct smb2_file_mode_info *)req->Buffer); + } }
- pr_err("Unimplemented Fileinfoclass :%d\n", info_class); + pr_err("Unimplemented Fileinfoclass :%d\n", req->FileInfoClass); return -EOPNOTSUPP; }
@@ -5774,8 +5840,7 @@ int smb2_set_info(struct ksmbd_work *work) switch (req->InfoType) { case SMB2_O_INFO_FILE: ksmbd_debug(SMB, "GOT SMB2_O_INFO_FILE\n"); - rc = smb2_set_info_file(work, fp, req->FileInfoClass, - req->Buffer, work->tcon->share_conf); + rc = smb2_set_info_file(work, fp, req, work->tcon->share_conf); break; case SMB2_O_INFO_SECURITY: ksmbd_debug(SMB, "GOT SMB2_O_INFO_SECURITY\n");