git: d068ea16e326 - main - cam: Let cam_periph_unmapmem() return an error
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 28 Dec 2023 17:36:34 UTC
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=d068ea16e3264c2d62472a8acf794262cfe703dd
commit d068ea16e3264c2d62472a8acf794262cfe703dd
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-12-28 17:08:04 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-12-28 17:36:15 +0000
cam: Let cam_periph_unmapmem() return an error
As of commit b059686a71c8, cam_periph_unmapmem() can legitimately fail
if the copyout() operation fails. However, this failure was never
signaled to upper layers. In practice it is unlikely to occur
since cap_periph_mapmem() would most likely fail in such
circumstances anyway, but an error is nonetheless possible.
However, some code reading revealed a few paths where the return value
of cam_periph_mapmem() is not checked, and this is definitely a bug.
Add error checking there and let cam_periph_unmapmem() return errors
from copyout().
Reviewed by: dab, mav
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D43201
---
sys/cam/cam_compat.c | 21 +++++++++++++--------
sys/cam/cam_periph.c | 21 +++++++++++++--------
sys/cam/cam_periph.h | 2 +-
sys/cam/cam_xpt.c | 4 +---
sys/cam/nvme/nvme_da.c | 5 +++--
sys/cam/scsi/scsi_pass.c | 4 ++--
sys/cam/scsi/scsi_sg.c | 6 ++++--
sys/cam/scsi/scsi_target.c | 29 ++++++++++++++++++++---------
8 files changed, 57 insertions(+), 35 deletions(-)
diff --git a/sys/cam/cam_compat.c b/sys/cam/cam_compat.c
index 1b53eaa4a2c8..fdb4ee8717ec 100644
--- a/sys/cam/cam_compat.c
+++ b/sys/cam/cam_compat.c
@@ -377,11 +377,13 @@ cam_compat_translate_dev_match_0x18(union ccb *ccb)
struct dev_match_result *dm;
struct dev_match_result_0x18 *dm18;
struct cam_periph_map_info mapinfo;
- int i;
+ int error, i;
/* Remap the CCB into kernel address space */
bzero(&mapinfo, sizeof(mapinfo));
- cam_periph_mapmem(ccb, &mapinfo, maxphys);
+ error = cam_periph_mapmem(ccb, &mapinfo, maxphys);
+ if (error != 0)
+ return (error);
dm = ccb->cdm.matches;
/* Translate in-place: old fields are smaller */
@@ -429,21 +431,22 @@ cam_compat_translate_dev_match_0x18(union ccb *ccb)
}
}
- cam_periph_unmapmem(ccb, &mapinfo);
-
- return (0);
+ return (cam_periph_unmapmem(ccb, &mapinfo));
}
static int
cam_compat_handle_0x19(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
struct thread *td, d_ioctl_t *cbfnp)
{
+ struct cam_periph_map_info mapinfo;
union ccb *ccb = (union ccb *)addr;
- struct cam_periph_map_info mapinfo;
+ int error;
if (cmd == CAMIOCOMMAND && ccb->ccb_h.func_code == XPT_DEV_MATCH) {
bzero(&mapinfo, sizeof(mapinfo));
- cam_periph_mapmem(ccb, &mapinfo, maxphys);
+ error = cam_periph_mapmem(ccb, &mapinfo, maxphys);
+ if (error != 0)
+ return (error);
for (int i = 0; i < ccb->cdm.num_patterns; i++) {
struct dev_match_pattern *p = &ccb->cdm.patterns[i];
@@ -457,7 +460,9 @@ cam_compat_handle_0x19(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
p->pattern.periph_pattern.flags == 0x01f)
p->pattern.periph_pattern.flags = PERIPH_MATCH_ANY;
}
- cam_periph_unmapmem(ccb, &mapinfo);
+ error = cam_periph_unmapmem(ccb, &mapinfo);
+ if (error != 0)
+ return (error);
}
return ((cbfnp)(dev, cmd, addr, flag, td));
}
diff --git a/sys/cam/cam_periph.c b/sys/cam/cam_periph.c
index 920e5f0471c7..e957edee67f1 100644
--- a/sys/cam/cam_periph.c
+++ b/sys/cam/cam_periph.c
@@ -1013,17 +1013,17 @@ fail:
* Unmap memory segments mapped into kernel virtual address space by
* cam_periph_mapmem().
*/
-void
+int
cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
{
- int numbufs, i;
+ int error, numbufs, i;
uint8_t **data_ptrs[CAM_PERIPH_MAXMAPS];
uint32_t lengths[CAM_PERIPH_MAXMAPS];
uint32_t dirs[CAM_PERIPH_MAXMAPS];
if (mapinfo->num_bufs_used <= 0) {
/* nothing to free and the process wasn't held. */
- return;
+ return (0);
}
switch (ccb->ccb_h.func_code) {
@@ -1088,12 +1088,11 @@ cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
numbufs = 1;
break;
default:
- /* allow ourselves to be swapped once again */
- PRELE(curproc);
- return;
- break; /* NOTREACHED */
+ numbufs = 0;
+ break;
}
+ error = 0;
for (i = 0; i < numbufs; i++) {
if (mapinfo->bp[i]) {
/* unmap the buffer */
@@ -1103,8 +1102,12 @@ cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
uma_zfree(pbuf_zone, mapinfo->bp[i]);
} else {
if (dirs[i] != CAM_DIR_OUT) {
- copyout(*data_ptrs[i], mapinfo->orig[i],
+ int error1;
+
+ error1 = copyout(*data_ptrs[i], mapinfo->orig[i],
lengths[i]);
+ if (error == 0)
+ error = error1;
}
free(*data_ptrs[i], M_CAMPERIPH);
}
@@ -1115,6 +1118,8 @@ cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
/* allow ourselves to be swapped once again */
PRELE(curproc);
+
+ return (error);
}
int
diff --git a/sys/cam/cam_periph.h b/sys/cam/cam_periph.h
index d0f04388b181..ad18ee6b204c 100644
--- a/sys/cam/cam_periph.h
+++ b/sys/cam/cam_periph.h
@@ -180,7 +180,7 @@ void cam_periph_invalidate(struct cam_periph *periph);
int cam_periph_mapmem(union ccb *ccb,
struct cam_periph_map_info *mapinfo,
u_int maxmap);
-void cam_periph_unmapmem(union ccb *ccb,
+int cam_periph_unmapmem(union ccb *ccb,
struct cam_periph_map_info *mapinfo);
union ccb *cam_periph_getccb(struct cam_periph *periph,
uint32_t priority);
diff --git a/sys/cam/cam_xpt.c b/sys/cam/cam_xpt.c
index de3630c35374..2acb106b087c 100644
--- a/sys/cam/cam_xpt.c
+++ b/sys/cam/cam_xpt.c
@@ -563,11 +563,9 @@ xptdoioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag, struct thread *
/*
* Map the buffers back into user space.
*/
- cam_periph_unmapmem(inccb, &mapinfo);
+ error = cam_periph_unmapmem(inccb, &mapinfo);
inccb->ccb_h.path = old_path;
-
- error = 0;
break;
}
default:
diff --git a/sys/cam/nvme/nvme_da.c b/sys/cam/nvme/nvme_da.c
index 73c385b2fe1c..2dd7c6d4a70b 100644
--- a/sys/cam/nvme/nvme_da.c
+++ b/sys/cam/nvme/nvme_da.c
@@ -437,8 +437,9 @@ ndaioctl(struct disk *dp, u_long cmd, void *data, int fflag,
* Tear down mapping and return status.
*/
cam_periph_unlock(periph);
- cam_periph_unmapmem(ccb, &mapinfo);
- error = cam_ccb_success(ccb) ? 0 : EIO;
+ error = cam_periph_unmapmem(ccb, &mapinfo);
+ if (!cam_ccb_success(ccb))
+ error = EIO;
out:
cam_periph_lock(periph);
xpt_release_ccb(ccb);
diff --git a/sys/cam/scsi/scsi_pass.c b/sys/cam/scsi/scsi_pass.c
index 5c05ca84bcab..5a24f11e60e2 100644
--- a/sys/cam/scsi/scsi_pass.c
+++ b/sys/cam/scsi/scsi_pass.c
@@ -2236,14 +2236,14 @@ passsendccb(struct cam_periph *periph, union ccb *ccb, union ccb *inccb)
}
cam_periph_unlock(periph);
- cam_periph_unmapmem(ccb, &mapinfo);
+ error = cam_periph_unmapmem(ccb, &mapinfo);
cam_periph_lock(periph);
ccb->ccb_h.cbfcnp = NULL;
ccb->ccb_h.periph_priv = inccb->ccb_h.periph_priv;
bcopy(ccb, inccb, sizeof(union ccb));
- return(0);
+ return (error);
}
/*
diff --git a/sys/cam/scsi/scsi_sg.c b/sys/cam/scsi/scsi_sg.c
index 27a0b298fd31..fec3bb157b52 100644
--- a/sys/cam/scsi/scsi_sg.c
+++ b/sys/cam/scsi/scsi_sg.c
@@ -882,7 +882,7 @@ sgsendccb(struct cam_periph *periph, union ccb *ccb)
{
struct sg_softc *softc;
struct cam_periph_map_info mapinfo;
- int error;
+ int error, error1;
softc = periph->softc;
bzero(&mapinfo, sizeof(mapinfo));
@@ -907,7 +907,9 @@ sgsendccb(struct cam_periph *periph, union ccb *ccb)
softc->device_stats);
cam_periph_unlock(periph);
- cam_periph_unmapmem(ccb, &mapinfo);
+ error1 = cam_periph_unmapmem(ccb, &mapinfo);
+ if (error == 0)
+ error = error1;
cam_periph_lock(periph);
return (error);
diff --git a/sys/cam/scsi/scsi_target.c b/sys/cam/scsi/scsi_target.c
index 5fe123427447..f1c01cab8638 100644
--- a/sys/cam/scsi/scsi_target.c
+++ b/sys/cam/scsi/scsi_target.c
@@ -905,18 +905,29 @@ targreturnccb(struct targ_softc *softc, union ccb *ccb)
u_ccbh = &descr->user_ccb->ccb_h;
/* Copy out the central portion of the ccb_hdr */
- copyout(&ccb->ccb_h.retry_count, &u_ccbh->retry_count,
- offsetof(struct ccb_hdr, periph_priv) -
- offsetof(struct ccb_hdr, retry_count));
+ error = copyout(&ccb->ccb_h.retry_count, &u_ccbh->retry_count,
+ offsetof(struct ccb_hdr, periph_priv) -
+ offsetof(struct ccb_hdr, retry_count));
+ if (error != 0) {
+ xpt_print(softc->path,
+ "targreturnccb - CCB header copyout failed (%d)\n", error);
+ }
/* Copy out the rest of the ccb (after the ccb_hdr) */
ccb_len = targccblen(ccb->ccb_h.func_code) - sizeof(struct ccb_hdr);
- if (descr->mapinfo.num_bufs_used != 0)
- cam_periph_unmapmem(ccb, &descr->mapinfo);
- error = copyout(&ccb->ccb_h + 1, u_ccbh + 1, ccb_len);
- if (error != 0) {
- xpt_print(softc->path,
- "targreturnccb - CCB copyout failed (%d)\n", error);
+ if (descr->mapinfo.num_bufs_used != 0) {
+ int error1;
+
+ error1 = cam_periph_unmapmem(ccb, &descr->mapinfo);
+ if (error == 0)
+ error = error1;
+ }
+ if (error == 0) {
+ error = copyout(&ccb->ccb_h + 1, u_ccbh + 1, ccb_len);
+ if (error != 0) {
+ xpt_print(softc->path,
+ "targreturnccb - CCB copyout failed (%d)\n", error);
+ }
}
/* Free CCB or send back to devq. */
targfreeccb(softc, ccb);