From owner-svn-src-stable-11@freebsd.org Thu May 14 17:56:46 2020 Return-Path: Delivered-To: svn-src-stable-11@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 195092D9205; Thu, 14 May 2020 17:56:46 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49NK3x6tFlz4d99; Thu, 14 May 2020 17:56:45 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id E7593EF91; Thu, 14 May 2020 17:56:45 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 04EHujEo085366; Thu, 14 May 2020 17:56:45 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 04EHujD4085362; Thu, 14 May 2020 17:56:45 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <202005141756.04EHujD4085362@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Thu, 14 May 2020 17:56:45 +0000 (UTC) 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 X-SVN-Group: stable-11 X-SVN-Commit-Author: jhb X-SVN-Commit-Paths: 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 X-SVN-Commit-Revision: 361038 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 May 2020 17:56:46 -0000 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;