Hello Rob Clark,
The patch 20224d715a88: "drm/msm/submit: Move copy_from_user ahead of locking bos" from Oct 23, 2020, leads to the following Smatch static checker warning:
drivers/gpu/drm/msm/msm_gem_submit.c:207 submit_lookup_cmds() warn: impossible condition '(sz == (~0)) => (0-u32max == u64max)'
drivers/gpu/drm/msm/msm_gem_submit.c 161 static int submit_lookup_cmds(struct msm_gem_submit *submit, 162 struct drm_msm_gem_submit *args, struct drm_file *file) 163 { 164 unsigned i, sz; 165 int ret = 0; 166 167 for (i = 0; i < args->nr_cmds; i++) { 168 struct drm_msm_gem_submit_cmd submit_cmd; 169 void __user *userptr = 170 u64_to_user_ptr(args->cmds + (i * sizeof(submit_cmd))); 171 172 ret = copy_from_user(&submit_cmd, userptr, sizeof(submit_cmd)); 173 if (ret) { 174 ret = -EFAULT; 175 goto out; 176 } 177 178 /* validate input from userspace: */ 179 switch (submit_cmd.type) { 180 case MSM_SUBMIT_CMD_BUF: 181 case MSM_SUBMIT_CMD_IB_TARGET_BUF: 182 case MSM_SUBMIT_CMD_CTX_RESTORE_BUF: 183 break; 184 default: 185 DRM_ERROR("invalid type: %08x\n", submit_cmd.type); 186 return -EINVAL; 187 } 188 189 if (submit_cmd.size % 4) { 190 DRM_ERROR("non-aligned cmdstream buffer size: %u\n", 191 submit_cmd.size); 192 ret = -EINVAL; 193 goto out; 194 } 195 196 submit->cmd[i].type = submit_cmd.type; 197 submit->cmd[i].size = submit_cmd.size / 4; 198 submit->cmd[i].offset = submit_cmd.submit_offset / 4; 199 submit->cmd[i].idx = submit_cmd.submit_idx; 200 submit->cmd[i].nr_relocs = submit_cmd.nr_relocs; 201 202 userptr = u64_to_user_ptr(submit_cmd.relocs); 203 204 sz = array_size(submit_cmd.nr_relocs, 205 sizeof(struct drm_msm_gem_submit_reloc)); 206 /* check for overflow: */ --> 207 if (sz == SIZE_MAX) {
"sz" is u32 (not size_t) so this check is impossible on 64 bit systems. May as well just remove it and let the kmalloc() fail. Or use a smaller limit so that users can't trigger the kmalloc() failure.
208 ret = -ENOMEM; 209 goto out; 210 } 211 submit->cmd[i].relocs = kmalloc(sz, GFP_KERNEL); 212 ret = copy_from_user(submit->cmd[i].relocs, userptr, sz); 213 if (ret) { 214 ret = -EFAULT; 215 goto out; 216 } 217 } 218 219 out: 220 return ret; 221 }
regards, dan carpenter