Checks here against useracc are not useful and are racy.

copyin/copyout are sufficient to guard against bad addresses. They will return
EFAULT if the user is up to no good (by choice or ignorance). There's no point
in checking, since it doesn't even improve the error messages.

Noticed by: jhb
Reviewed by: brooks, jhb
This commit is contained in:
imp 2020-04-13 21:04:33 +00:00
parent 01b6a4097c
commit dab0442b3e

View File

@ -1232,15 +1232,6 @@ passcopysglist(struct cam_periph *periph, struct pass_io_req *io_req,
user_watermark += len_to_copy;
kern_watermark += len_to_copy;
if (!useracc(user_ptr, len_to_copy,
(direction == CAM_DIR_IN) ? VM_PROT_WRITE : VM_PROT_READ)) {
xpt_print(periph->path, "%s: unable to access user "
"S/G list element %p len %zu\n", __func__,
user_ptr, len_to_copy);
error = EFAULT;
goto bailout;
}
if (direction == CAM_DIR_IN) {
error = copyout(kern_ptr, user_ptr, len_to_copy);
if (error != 0) {
@ -1445,20 +1436,6 @@ passmemsetup(struct cam_periph *periph, struct pass_io_req *io_req)
if (io_req->lengths[i] == 0)
continue;
/*
* Make sure that the user's buffer is accessible
* to that process.
*/
if (!useracc(io_req->user_bufs[i], io_req->lengths[i],
(io_req->dirs[i] == CAM_DIR_IN) ? VM_PROT_WRITE :
VM_PROT_READ)) {
xpt_print(periph->path, "%s: user address %p "
"length %u is not accessible\n", __func__,
io_req->user_bufs[i], io_req->lengths[i]);
error = EFAULT;
goto bailout;
}
tmp_buf = malloc(lengths[i], M_SCSIPASS,
M_WAITOK | M_ZERO);
io_req->kern_bufs[i] = tmp_buf;
@ -1561,13 +1538,6 @@ passmemsetup(struct cam_periph *periph, struct pass_io_req *io_req)
} else
io_req->user_segptr = io_req->user_segs;
if (!useracc(*data_ptrs[0], sg_length, VM_PROT_READ)) {
xpt_print(periph->path, "%s: unable to access user "
"S/G list at %p\n", __func__, *data_ptrs[0]);
error = EFAULT;
goto bailout;
}
error = copyin(*data_ptrs[0], io_req->user_segptr, sg_length);
if (error != 0) {
xpt_print(periph->path, "%s: copy of user S/G list "