be072b06c73970 ("drm/amdkfd: CRIU export BOs as prime dmabuf objects")
added an error path in criu_checkpoint() that (unless I'm completely
misreading this) treats the userspace-supplied args->bos (which was
previously used as a userspace pointer when passed to
criu_checkpoint_bos()) as a kernel pointer:
ret = criu_checkpoint_bos(p, num_bos, (uint8_t __user *)args->bos,
(uint8_t __user *)args->priv_data, &priv_offset);
if (ret)
goto exit_unlock;
[...]
close_bo_fds:
if (ret) {
/* If IOCTL returns err, user assumes all FDs opened in
criu_dump_bos are closed */
uint32_t i;
struct kfd_criu_bo_bucket *bo_buckets = (struct kfd_criu_bo_bucket
*) args->bos;
for (i = 0; i < num_bos; i++) {
if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
close_fd(bo_buckets[i].dmabuf_fd);
}
}
This seems very wrong, and also like it's guaranteed to blow up as
soon as it runs on a machine with SMAP, which makes me think that this
codepath was probably never exercised?
(Also note that just changing this to copy_from_user() instead would
still be wrong, because malicious/bogus userspace could change the FD
number to the KFD device's FD, and the VFS assumes that an FD can't be
closed while it's being accessed in a single-threaded process.)
