Date: Thu, 14 May 2020 17:56:45 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r361038 - in stable: 11/sys/cam/scsi 11/sys/dev/iscsi_initiator 11/sys/net80211 12/sys/cam/scsi 12/sys/dev/iscsi_initiator 12/sys/net80211 Message-ID: <202005141756.04EHujD4085362@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Thu May 14 17:56:43 2020 New Revision: 361038 URL: https://svnweb.freebsd.org/changeset/base/361038 Log: MFC 360171,360179,360285,360388: 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. Modified: stable/11/sys/cam/scsi/scsi_enc_ses.c stable/11/sys/cam/scsi/scsi_sg.c stable/11/sys/dev/iscsi_initiator/isc_subr.c stable/11/sys/net80211/ieee80211_mesh.c Directory Properties: stable/11/ (props changed) Changes in other areas also in this revision: Modified: stable/12/sys/cam/scsi/scsi_enc_ses.c stable/12/sys/cam/scsi/scsi_sg.c stable/12/sys/dev/iscsi_initiator/isc_subr.c stable/12/sys/net80211/ieee80211_mesh.c Directory Properties: stable/12/ (props changed) Modified: stable/11/sys/cam/scsi/scsi_enc_ses.c ============================================================================== --- stable/11/sys/cam/scsi/scsi_enc_ses.c Thu May 14 17:54:08 2020 (r361037) +++ stable/11/sys/cam/scsi/scsi_enc_ses.c Thu May 14 17:56:43 2020 (r361038) @@ -2902,13 +2902,19 @@ ses_handle_string(enc_softc_t *enc, encioc_string_t *s 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,6 +2954,8 @@ ses_handle_string(enc_softc_t *enc, encioc_string_t *s return (EINVAL); } ret = enc_runcmd(enc, cdb, 6, buf, &amt); + if (ret == 0 && ioc == ENCIOC_GETSTRING) + ret = copyout(buf, sstr->buf, sstr->bufsiz); if (ioc == ENCIOC_SETSTRING) ENC_FREE(buf); return (ret); Modified: stable/11/sys/cam/scsi/scsi_sg.c ============================================================================== --- stable/11/sys/cam/scsi/scsi_sg.c Thu May 14 17:54:08 2020 (r361037) +++ stable/11/sys/cam/scsi/scsi_sg.c Thu May 14 17:56:43 2020 (r361038) @@ -506,6 +506,7 @@ sgioctl(struct cdev *dev, u_long cmd, caddr_t arg, int 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 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 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 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 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 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; Modified: stable/11/sys/dev/iscsi_initiator/isc_subr.c ============================================================================== --- stable/11/sys/dev/iscsi_initiator/isc_subr.c Thu May 14 17:54:08 2020 (r361037) +++ stable/11/sys/dev/iscsi_initiator/isc_subr.c Thu May 14 17:56:43 2020 (r361038) @@ -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"); } Modified: stable/11/sys/net80211/ieee80211_mesh.c ============================================================================== --- stable/11/sys/net80211/ieee80211_mesh.c Thu May 14 17:54:08 2020 (r361037) +++ stable/11/sys/net80211/ieee80211_mesh.c Thu May 14 17:56:43 2020 (r361038) @@ -3567,16 +3567,21 @@ mesh_ioctl_set80211(struct ieee80211vap *vap, struct i 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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202005141756.04EHujD4085362>