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
