aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohn Baldwin <jhb@FreeBSD.org>2020-05-14 18:50:43 +0000
committerJohn Baldwin <jhb@FreeBSD.org>2020-05-14 18:50:43 +0000
commit62863d44bbe90719bf229f63402910def81f49cd (patch)
tree84bd73b1c4b2c89ad72e3fa079c0229c4da3290f
parent7be8492d5e12242d9a43ec0bbee40fb60fb762df (diff)
downloadsrc-62863d44bbe90719bf229f63402910def81f49cd.tar.gz
src-62863d44bbe90719bf229f63402910def81f49cd.zip
MF11 361038,361040: Don't dereference various user pointers.
360171: Don't access a user buffer directly from the kernel. The handle_string callback for the ENCIOC_SETSTRING ioctl was passing a user pointer to memcpy(). Fix by using copyin() instead. For ENCIOC_GETSTRING ioctls, the handler was storing the user pointer in a CCB's data_ptr field where it was indirected by other code. Fix this by allocating a temporary buffer (which ENCIOC_SETSTRING already did) and copying the result out to the user buffer after the CCB has been processed. 360179: Don't pass a user buffer pointer as the data pointer in a CCB. Allocate a temporary buffer in the kernel to serve as the CCB data pointer for a pass-through transaction and use copyin/copyout to shuffle the data to/from the user buffer. 360285: Don't indirect user pointers directly in two 802.11s ioctls. IEEE80211_MESH_RTCMD_ADD was invoking memcmp() to validate the supplied address directly on the user pointer rather than first doing a copyin() and validating the copied value. IEEE80211_MESH_RTCMD_DELETE was passing the user pointer directly to ieee80211_mesh_rt_del() rather than copying the user buffer into a temporary kernel buffer. 360388: Don't run strcmp() against strings stored in user memory. Instead, copy the strings into a temporary buffer on the stack and run strcmp on the copies. 360818: Fix a memory leak for ENCIOC_GETSTRING I introduced in r360171. Approved by: re (gjb)
Notes
Notes: svn path=/releng/11.4/; revision=361042
-rw-r--r--sys/cam/scsi/scsi_enc_ses.c14
-rw-r--r--sys/cam/scsi/scsi_sg.c27
-rw-r--r--sys/dev/iscsi_initiator/isc_subr.c17
-rw-r--r--sys/net80211/ieee80211_mesh.c19
4 files changed, 61 insertions, 16 deletions
diff --git a/sys/cam/scsi/scsi_enc_ses.c b/sys/cam/scsi/scsi_enc_ses.c
index 49feb17e81c2..382899406f19 100644
--- a/sys/cam/scsi/scsi_enc_ses.c
+++ b/sys/cam/scsi/scsi_enc_ses.c
@@ -2902,13 +2902,19 @@ ses_handle_string(enc_softc_t *enc, encioc_string_t *sstr, int ioc)
buf[1] = 0;
buf[2] = sstr->bufsiz >> 8;
buf[3] = sstr->bufsiz & 0xff;
- memcpy(&buf[4], sstr->buf, sstr->bufsiz);
+ ret = copyin(sstr->buf, &buf[4], sstr->bufsiz);
+ if (ret != 0) {
+ ENC_FREE(buf);
+ return (ret);
+ }
break;
case ENCIOC_GETSTRING:
payload = sstr->bufsiz;
amt = payload;
+ buf = ENC_MALLOC(payload);
+ if (buf == NULL)
+ return (ENOMEM);
ses_page_cdb(cdb, payload, SesStringIn, CAM_DIR_IN);
- buf = sstr->buf;
break;
case ENCIOC_GETENCNAME:
if (ses_cache->ses_nsubencs < 1)
@@ -2948,7 +2954,9 @@ ses_handle_string(enc_softc_t *enc, encioc_string_t *sstr, int ioc)
return (EINVAL);
}
ret = enc_runcmd(enc, cdb, 6, buf, &amt);
- if (ioc == ENCIOC_SETSTRING)
+ if (ret == 0 && ioc == ENCIOC_GETSTRING)
+ ret = copyout(buf, sstr->buf, sstr->bufsiz);
+ if (ioc == ENCIOC_SETSTRING || ioc == ENCIOC_GETSTRING)
ENC_FREE(buf);
return (ret);
}
diff --git a/sys/cam/scsi/scsi_sg.c b/sys/cam/scsi/scsi_sg.c
index f330aedbe706..08c7489843a2 100644
--- a/sys/cam/scsi/scsi_sg.c
+++ b/sys/cam/scsi/scsi_sg.c
@@ -506,6 +506,7 @@ sgioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flag, struct thread *td)
struct cam_periph *periph;
struct sg_softc *softc;
struct sg_io_hdr *req;
+ void *data_ptr;
int dir, error;
periph = (struct cam_periph *)dev->si_drv1;
@@ -550,12 +551,20 @@ sgioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flag, struct thread *td)
break;
}
+ if (req->dxfer_len > MAXPHYS) {
+ error = EINVAL;
+ break;
+ }
+
+ data_ptr = malloc(req->dxfer_len, M_DEVBUF, M_WAITOK);
+
ccb = cam_periph_getccb(periph, CAM_PRIORITY_NORMAL);
csio = &ccb->csio;
error = copyin(req->cmdp, &csio->cdb_io.cdb_bytes,
req->cmd_len);
if (error) {
+ free(data_ptr, M_DEVBUF);
xpt_release_ccb(ccb);
break;
}
@@ -568,7 +577,7 @@ sgioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flag, struct thread *td)
dir = CAM_DIR_IN;
break;
case SG_DXFER_TO_FROM_DEV:
- dir = CAM_DIR_IN | CAM_DIR_OUT;
+ dir = CAM_DIR_BOTH;
break;
case SG_DXFER_NONE:
default:
@@ -576,12 +585,21 @@ sgioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flag, struct thread *td)
break;
}
+ if (dir == CAM_DIR_IN || dir == CAM_DIR_BOTH) {
+ error = copyin(req->dxferp, data_ptr, req->dxfer_len);
+ if (error) {
+ free(data_ptr, M_DEVBUF);
+ xpt_release_ccb(ccb);
+ break;
+ }
+ }
+
cam_fill_csio(csio,
/*retries*/1,
sgdone,
dir|CAM_DEV_QFRZDIS,
MSG_SIMPLE_Q_TAG,
- req->dxferp,
+ data_ptr,
req->dxfer_len,
req->mx_sb_len,
req->cmd_len,
@@ -591,6 +609,7 @@ sgioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flag, struct thread *td)
if (error) {
req->host_status = DID_ERROR;
req->driver_status = DRIVER_INVALID;
+ free(data_ptr, M_DEVBUF);
xpt_release_ccb(ccb);
break;
}
@@ -609,6 +628,10 @@ sgioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flag, struct thread *td)
req->sb_len_wr);
}
+ if ((dir == CAM_DIR_OUT || dir == CAM_DIR_BOTH) && error == 0)
+ error = copyout(data_ptr, req->dxferp, req->dxfer_len);
+
+ free(data_ptr, M_DEVBUF);
xpt_release_ccb(ccb);
break;
diff --git a/sys/dev/iscsi_initiator/isc_subr.c b/sys/dev/iscsi_initiator/isc_subr.c
index d553cd1c3326..9c812494c298 100644
--- a/sys/dev/iscsi_initiator/isc_subr.c
+++ b/sys/dev/iscsi_initiator/isc_subr.c
@@ -192,6 +192,9 @@ i_crc32c(const void *buf, size_t size, uint32_t crc)
int
i_setopt(isc_session_t *sp, isc_opt_t *opt)
{
+ char buf[16];
+ int error;
+
if(opt->maxRecvDataSegmentLength > 0) {
sp->opt.maxRecvDataSegmentLength = opt->maxRecvDataSegmentLength;
sdebug(2, "maxRecvDataSegmentLength=%d", sp->opt.maxRecvDataSegmentLength);
@@ -233,15 +236,21 @@ i_setopt(isc_session_t *sp, isc_opt_t *opt)
}
if(opt->headerDigest != NULL) {
- sdebug(2, "opt.headerDigest='%s'", opt->headerDigest);
- if(strcmp(opt->headerDigest, "CRC32C") == 0) {
+ error = copyinstr(opt->headerDigest, buf, sizeof(buf), NULL);
+ if (error != 0)
+ return (error);
+ sdebug(2, "opt.headerDigest='%s'", buf);
+ if(strcmp(buf, "CRC32C") == 0) {
sp->hdrDigest = (digest_t *)i_crc32c;
sdebug(2, "opt.headerDigest set");
}
}
if(opt->dataDigest != NULL) {
- sdebug(2, "opt.dataDigest='%s'", opt->headerDigest);
- if(strcmp(opt->dataDigest, "CRC32C") == 0) {
+ error = copyinstr(opt->dataDigest, buf, sizeof(buf), NULL);
+ if (error != 0)
+ return (error);
+ sdebug(2, "opt.dataDigest='%s'", opt->dataDigest);
+ if(strcmp(buf, "CRC32C") == 0) {
sp->dataDigest = (digest_t *)i_crc32c;
sdebug(2, "opt.dataDigest set");
}
diff --git a/sys/net80211/ieee80211_mesh.c b/sys/net80211/ieee80211_mesh.c
index 640bc33aa141..4c344bcd60e5 100644
--- a/sys/net80211/ieee80211_mesh.c
+++ b/sys/net80211/ieee80211_mesh.c
@@ -3567,16 +3567,21 @@ mesh_ioctl_set80211(struct ieee80211vap *vap, struct ieee80211req *ireq)
ieee80211_mesh_rt_flush(vap);
break;
case IEEE80211_MESH_RTCMD_ADD:
- if (IEEE80211_ADDR_EQ(vap->iv_myaddr, ireq->i_data) ||
- IEEE80211_ADDR_EQ(broadcastaddr, ireq->i_data))
- return EINVAL;
- error = copyin(ireq->i_data, &tmpaddr,
+ error = copyin(ireq->i_data, tmpaddr,
IEEE80211_ADDR_LEN);
- if (error == 0)
- ieee80211_mesh_discover(vap, tmpaddr, NULL);
+ if (error != 0)
+ break;
+ if (IEEE80211_ADDR_EQ(vap->iv_myaddr, tmpaddr) ||
+ IEEE80211_ADDR_EQ(broadcastaddr, tmpaddr))
+ return EINVAL;
+ ieee80211_mesh_discover(vap, tmpaddr, NULL);
break;
case IEEE80211_MESH_RTCMD_DELETE:
- ieee80211_mesh_rt_del(vap, ireq->i_data);
+ error = copyin(ireq->i_data, tmpaddr,
+ IEEE80211_ADDR_LEN);
+ if (error != 0)
+ break;
+ ieee80211_mesh_rt_del(vap, tmpaddr);
break;
default:
return ENOSYS;