[bug report] cifsd: add file operations

[bug report] cifsd: add file operations

[Dan Carpenter]

Hello Namjae Jeon,

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

	fs/ksmbd/vfs_cache.c:661 ksmbd_file_table_flush()
	warn: was expecting a 64 bit value instead of '0'

fs/ksmbd/vfs_cache.c
   653  int ksmbd_file_table_flush(struct ksmbd_work *work)
   654  {
   655          struct ksmbd_file       *fp = NULL;
   656          unsigned int            id;
   657          int                     ret;
   658  
   659          read_lock(&work->sess->file_table.lock);
   660          idr_for_each_entry(work->sess->file_table.idr, fp, id) {
   661                  ret = ksmbd_vfs_fsync(work, fp->volatile_id, KSMBD_NO_FID);

TBH, I'm not sure what's triggering this warning, but ksmbd_vfs_fsync()
takes a u64 and fp->volatile_id is a u32.  The ksmbd_lookup_fd_slow()
function takes a u32 so passing a u64 value seems like it would lead to
a bug.  And smb2_flush() passes a u64 so this seems buggy.

   662                  if (ret)
   663                          break;
   664          }
   665          read_unlock(&work->sess->file_table.lock);
   666          return ret;
   667  }

regards,
dan carpenter

[bug report] cifsd: add file operations

[Dan Carpenter]

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