diff --git a/sys/dev/raid/mps/mps_user.c b/sys/dev/raid/mps/mps_user.c --- a/sys/dev/raid/mps/mps_user.c +++ b/sys/dev/raid/mps/mps_user.c @@ -798,15 +798,24 @@ /* * copy in the header so we know what we're dealing with before we * commit to allocating a command for it. + * + * DF-1359: 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->facts->IOCRequestFrameSize * 4) { + if (data->RequestSize < (int)sizeof(tmphdr) || + data->RequestSize > (int)sc->facts->IOCRequestFrameSize * 4) { err = EINVAL; goto RetFreeUnlocked; } + err = copyin(PTRIN(data->PtrRequest), &tmphdr, sizeof(tmphdr)); + if (err != 0) + goto RetFreeUnlocked; function = tmphdr.Function; mps_dprint(sc, MPS_INFO, "%s: Function %02X MsgFlags %02X\n", __func__, @@ -825,9 +834,17 @@ goto Ret; } - /* Copy the header in. Only a small fixup is needed. */ + /* DF-1359: 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. */ 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) { + mps_dprint(sc, MPS_FAULT, "%s: copyin task failed", + __func__); + goto Ret; + } task->TaskMID = cm->cm_desc.Default.SMID; cm->cm_data = NULL; @@ -875,7 +892,14 @@ mps_unlock(sc); hdr = (MPI2_REQUEST_HEADER *)cm->cm_req; - bcopy(&tmphdr, hdr, data->RequestSize); + /* DF-1359: 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) { + mps_dprint(sc, MPS_FAULT, "%s: copyin hdr failed", __func__); + goto Ret; + } /* * Do some checking to make sure the IOCTL request contains a valid