diff --git a/sys/dev/raid/mpr/mpr_user.c b/sys/dev/raid/mpr/mpr_user.c --- a/sys/dev/raid/mpr/mpr_user.c +++ b/sys/dev/raid/mpr/mpr_user.c @@ -796,15 +796,24 @@ /* * copy in the header so we know what we're dealing with before we * commit to allocating a command for it. + * + * DF-1326: tmphdr is a 12-byte MPI2_REQUEST_HEADER on this stack + * frame. An unchecked copyin of the user-controlled RequestSize + * overflows tmphdr (write past stack). The later bcopy of + * &tmphdr with the same RequestSize at the task/hdr sites also + * reads past tmphdr (read past stack). Bounds-check RequestSize + * BEFORE the copyin and only copy in the header bytes we will + * actually peek at here; the full request is re-read directly from + * userspace into the properly-sized cm->cm_req at the bcopy sites. */ - err = copyin(PTRIN(data->PtrRequest), &tmphdr, data->RequestSize); - if (err != 0) - goto RetFreeUnlocked; - - if (data->RequestSize > (int)sc->reqframesz) { + if (data->RequestSize < (int)sizeof(tmphdr) || + data->RequestSize > (int)sc->reqframesz) { err = EINVAL; goto RetFreeUnlocked; } + err = copyin(PTRIN(data->PtrRequest), &tmphdr, sizeof(tmphdr)); + if (err != 0) + goto RetFreeUnlocked; function = tmphdr.Function; mpr_dprint(sc, MPR_USER, "%s: Function %02X MsgFlags %02X\n", __func__, @@ -823,9 +832,17 @@ goto Ret; } - /* Copy the header in. Only a small fixup is needed. */ + /* Copy the full request directly from userspace into the + * properly-sized HW command buffer. Reading from &tmphdr + * with data->RequestSize would read past the 12-byte stack + * header (DF-1326). */ task = (MPI2_SCSI_TASK_MANAGE_REQUEST *)cm->cm_req; - bcopy(&tmphdr, task, data->RequestSize); + err = copyin(PTRIN(data->PtrRequest), task, data->RequestSize); + if (err != 0) { + mpr_dprint(sc, MPR_FAULT, "%s: copyin task failed", + __func__); + goto Ret; + } task->TaskMID = cm->cm_desc.Default.SMID; cm->cm_data = NULL; @@ -883,7 +900,14 @@ mpr_unlock(sc); hdr = (MPI2_REQUEST_HEADER *)cm->cm_req; - bcopy(&tmphdr, hdr, data->RequestSize); + /* DF-1326: read the full request directly from userspace; the old + * bcopy(&tmphdr, hdr, RequestSize) read past the 12-byte stack + * header. */ + err = copyin(PTRIN(data->PtrRequest), hdr, data->RequestSize); + if (err != 0) { + mpr_dprint(sc, MPR_FAULT, "%s: copyin hdr failed", __func__); + goto Ret; + } /* * Do some checking to make sure the IOCTL request contains a valid