[bug report] cifsd: add file operations


Dan Carpenter <error27@...>
 

Hello Namjae Jeon,

The patch f44158485826: "cifsd: add file operations" from Mar 16,
2021, leads to the following Smatch static checker warning:

fs/ksmbd/vfs.c:1040 ksmbd_vfs_fqar_lseek() warn: no lower bound on 'length'
fs/ksmbd/vfs.c:1041 ksmbd_vfs_fqar_lseek() warn: no lower bound on 'start'

fs/ksmbd/vfs.c
1021 int ksmbd_vfs_fqar_lseek(struct ksmbd_file *fp, loff_t start, loff_t length,
1022 struct file_allocated_range_buffer *ranges,
1023 unsigned int in_count, unsigned int *out_count)
1024 {
1025 struct file *f = fp->filp;
1026 struct inode *inode = file_inode(fp->filp);
1027 loff_t maxbytes = (u64)inode->i_sb->s_maxbytes, end;
1028 loff_t extent_start, extent_end;
1029 int ret = 0;
1030
1031 if (start > maxbytes)
^^^^^^^^^^^^^^^^
loff_t is signed long long. This comes from the user via
fsctl_query_allocated_ranges(). Both start and length can be negative.
So we can end up search through negative offsets. Possibly
vfs_llseek() protects us.

1032 return -EFBIG;
1033
1034 if (!in_count)
1035 return 0;
1036
1037 /*
1038 * Shrink request scope to what the fs can actually handle.
1039 */
--> 1040 if (length > maxbytes || (maxbytes - length) < start)
1041 length = maxbytes - start;
1042
1043 if (start + length > inode->i_size)
1044 length = inode->i_size - start;
1045
1046 *out_count = 0;
1047 end = start + length;
1048 while (start < end && *out_count < in_count) {
1049 extent_start = vfs_llseek(f, start, SEEK_DATA);
1050 if (extent_start < 0) {
1051 if (extent_start != -ENXIO)
1052 ret = (int)extent_start;
1053 break;
1054 }
1055
1056 if (extent_start >= end)
1057 break;
1058
1059 extent_end = vfs_llseek(f, extent_start, SEEK_HOLE);
1060 if (extent_end < 0) {
1061 if (extent_end != -ENXIO)
1062 ret = (int)extent_end;
1063 break;
1064 } else if (extent_start >= extent_end) {
1065 break;
1066 }
1067
1068 ranges[*out_count].file_offset = cpu_to_le64(extent_start);
1069 ranges[(*out_count)++].length =
1070 cpu_to_le64(min(extent_end, end) - extent_start);
1071
1072 start = extent_end;
1073 }
1074
1075 return ret;
1076 }

regards,
dan carpenter