git: ed6fa9d618bf - stable/12 - mpr, mps: Fix a stack buffer overflow in the user passthru ioctl
Mark Johnston
markj at FreeBSD.org
Mon Jan 11 14:54:08 UTC 2021
The branch stable/12 has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=ed6fa9d618bff47dcd3fb000e5805e29d331578d
commit ed6fa9d618bff47dcd3fb000e5805e29d331578d
Author: Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-01-08 18:32:04 +0000
Commit: Mark Johnston <markj at FreeBSD.org>
CommitDate: 2021-01-11 14:43:37 +0000
mpr, mps: Fix a stack buffer overflow in the user passthru ioctl
Previously we copied in the request into a stack-allocated structure
that could be smaller than the request size. Furthermore, we checked
the request size only after doing the copyin.
Fix this by allocating a buffer to hold the request, then copying the
buffer's contents into a command descriptor. This is a bit heavy-handed
but I expect the overhead will not be noticeable. The approach of
coping the header in first is susceptible to TOCTOU problems.
Reviewed by: imp
Reported by: maxpl0it at protonmail.com
Differential Revision: https://reviews.freebsd.org/D27963
(cherry picked from commit de828a91db29fb20440e0d92f3d3136b314a9584)
---
sys/dev/mpr/mpr_user.c | 30 +++++++++++++++---------------
sys/dev/mps/mps_user.c | 32 ++++++++++++++++----------------
2 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/sys/dev/mpr/mpr_user.c b/sys/dev/mpr/mpr_user.c
index 1b2aa398b22b..3995d01154f7 100644
--- a/sys/dev/mpr/mpr_user.c
+++ b/sys/dev/mpr/mpr_user.c
@@ -737,11 +737,12 @@ RetFree:
static int
mpr_user_pass_thru(struct mpr_softc *sc, mpr_pass_thru_t *data)
{
- MPI2_REQUEST_HEADER *hdr, tmphdr;
+ MPI2_REQUEST_HEADER *hdr, *tmphdr;
MPI2_DEFAULT_REPLY *rpl;
Mpi26NVMeEncapsulatedErrorReply_t *nvme_error_reply = NULL;
Mpi26NVMeEncapsulatedRequest_t *nvme_encap_request = NULL;
struct mpr_command *cm = NULL;
+ void *req = NULL;
int i, err = 0, dir = 0, sz;
uint8_t tool, function = 0;
u_int sense_len;
@@ -793,22 +794,21 @@ mpr_user_pass_thru(struct mpr_softc *sc, mpr_pass_thru_t *data)
data->ReplySize, data->PtrData, data->DataSize,
data->PtrDataOut, data->DataOutSize, data->DataDirection);
- /*
- * copy in the header so we know what we're dealing with before we
- * commit to allocating a command for it.
- */
- err = copyin(PTRIN(data->PtrRequest), &tmphdr, data->RequestSize);
- if (err != 0)
- goto RetFreeUnlocked;
-
- if (data->RequestSize > (int)sc->reqframesz) {
+ if (data->RequestSize > sc->reqframesz) {
err = EINVAL;
goto RetFreeUnlocked;
}
- function = tmphdr.Function;
+ req = malloc(data->RequestSize, M_MPRUSER, M_WAITOK | M_ZERO);
+ tmphdr = (MPI2_REQUEST_HEADER *)req;
+
+ err = copyin(PTRIN(data->PtrRequest), req, data->RequestSize);
+ if (err != 0)
+ goto RetFreeUnlocked;
+
+ function = tmphdr->Function;
mpr_dprint(sc, MPR_USER, "%s: Function %02X MsgFlags %02X\n", __func__,
- function, tmphdr.MsgFlags);
+ function, tmphdr->MsgFlags);
/*
* Handle a passthru TM request.
@@ -825,7 +825,7 @@ mpr_user_pass_thru(struct mpr_softc *sc, mpr_pass_thru_t *data)
/* Copy the header in. Only a small fixup is needed. */
task = (MPI2_SCSI_TASK_MANAGE_REQUEST *)cm->cm_req;
- bcopy(&tmphdr, task, data->RequestSize);
+ memcpy(task, req, data->RequestSize);
task->TaskMID = cm->cm_desc.Default.SMID;
cm->cm_data = NULL;
@@ -872,7 +872,6 @@ mpr_user_pass_thru(struct mpr_softc *sc, mpr_pass_thru_t *data)
mpr_lock(sc);
cm = mpr_alloc_command(sc);
-
if (cm == NULL) {
mpr_printf(sc, "%s: no mpr requests\n", __func__);
err = ENOMEM;
@@ -881,7 +880,7 @@ mpr_user_pass_thru(struct mpr_softc *sc, mpr_pass_thru_t *data)
mpr_unlock(sc);
hdr = (MPI2_REQUEST_HEADER *)cm->cm_req;
- bcopy(&tmphdr, hdr, data->RequestSize);
+ memcpy(hdr, req, data->RequestSize);
/*
* Do some checking to make sure the IOCTL request contains a valid
@@ -1154,6 +1153,7 @@ RetFree:
Ret:
sc->mpr_flags &= ~MPR_FLAGS_BUSY;
mpr_unlock(sc);
+ free(req, M_MPRUSER);
return (err);
}
diff --git a/sys/dev/mps/mps_user.c b/sys/dev/mps/mps_user.c
index af1526c36d4c..ab4d1d2d86f3 100644
--- a/sys/dev/mps/mps_user.c
+++ b/sys/dev/mps/mps_user.c
@@ -677,7 +677,7 @@ mps_user_command(struct mps_softc *sc, struct mps_usr_command *cmd)
mps_dprint(sc, MPS_USER, "%s: req %p %d rpl %p %d\n", __func__,
cmd->req, cmd->req_len, cmd->rpl, cmd->rpl_len);
- if (cmd->req_len > (int)sc->reqframesz) {
+ if (cmd->req_len > sc->reqframesz) {
err = EINVAL;
goto RetFreeUnlocked;
}
@@ -750,9 +750,10 @@ RetFree:
static int
mps_user_pass_thru(struct mps_softc *sc, mps_pass_thru_t *data)
{
- MPI2_REQUEST_HEADER *hdr, tmphdr;
+ MPI2_REQUEST_HEADER *hdr, *tmphdr;
MPI2_DEFAULT_REPLY *rpl = NULL;
struct mps_command *cm = NULL;
+ void *req = NULL;
int err = 0, dir = 0, sz;
uint8_t function = 0;
u_int sense_len;
@@ -804,22 +805,21 @@ mps_user_pass_thru(struct mps_softc *sc, mps_pass_thru_t *data)
data->ReplySize, data->PtrData, data->DataSize,
data->PtrDataOut, data->DataOutSize, data->DataDirection);
- /*
- * copy in the header so we know what we're dealing with before we
- * commit to allocating a command for it.
- */
- err = copyin(PTRIN(data->PtrRequest), &tmphdr, data->RequestSize);
- if (err != 0)
- goto RetFreeUnlocked;
-
- if (data->RequestSize > (int)sc->reqframesz) {
+ if (data->RequestSize > sc->reqframesz) {
err = EINVAL;
goto RetFreeUnlocked;
}
- function = tmphdr.Function;
+ req = malloc(data->RequestSize, M_MPSUSER, M_WAITOK | M_ZERO);
+ tmphdr = (MPI2_REQUEST_HEADER *)req;
+
+ err = copyin(PTRIN(data->PtrRequest), req, data->RequestSize);
+ if (err != 0)
+ goto RetFreeUnlocked;
+
+ function = tmphdr->Function;
mps_dprint(sc, MPS_USER, "%s: Function %02X MsgFlags %02X\n", __func__,
- function, tmphdr.MsgFlags);
+ function, tmphdr->MsgFlags);
/*
* Handle a passthru TM request.
@@ -836,7 +836,7 @@ mps_user_pass_thru(struct mps_softc *sc, mps_pass_thru_t *data)
/* Copy the header in. Only a small fixup is needed. */
task = (MPI2_SCSI_TASK_MANAGE_REQUEST *)cm->cm_req;
- bcopy(&tmphdr, task, data->RequestSize);
+ memcpy(task, req, data->RequestSize);
task->TaskMID = cm->cm_desc.Default.SMID;
cm->cm_data = NULL;
@@ -883,7 +883,6 @@ mps_user_pass_thru(struct mps_softc *sc, mps_pass_thru_t *data)
mps_lock(sc);
cm = mps_alloc_command(sc);
-
if (cm == NULL) {
mps_printf(sc, "%s: no mps requests\n", __func__);
err = ENOMEM;
@@ -892,7 +891,7 @@ mps_user_pass_thru(struct mps_softc *sc, mps_pass_thru_t *data)
mps_unlock(sc);
hdr = (MPI2_REQUEST_HEADER *)cm->cm_req;
- bcopy(&tmphdr, hdr, data->RequestSize);
+ memcpy(hdr, req, data->RequestSize);
/*
* Do some checking to make sure the IOCTL request contains a valid
@@ -1059,6 +1058,7 @@ RetFreeUnlocked:
Ret:
sc->mps_flags &= ~MPS_FLAGS_BUSY;
mps_unlock(sc);
+ free(req, M_MPSUSER);
return (err);
}
More information about the dev-commits-src-all
mailing list