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/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)

Changes in other areas also in this revision:
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)

Modified: stable/12/sys/cam/scsi/scsi_enc_ses.c
==============================================================================
--- stable/12/sys/cam/scsi/scsi_enc_ses.c       Thu May 14 17:54:08 2020        
(r361037)
+++ stable/12/sys/cam/scsi/scsi_enc_ses.c       Thu May 14 17:56:43 2020        
(r361038)
@@ -2904,13 +2904,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)
@@ -2950,6 +2956,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/12/sys/cam/scsi/scsi_sg.c
==============================================================================
--- stable/12/sys/cam/scsi/scsi_sg.c    Thu May 14 17:54:08 2020        
(r361037)
+++ stable/12/sys/cam/scsi/scsi_sg.c    Thu May 14 17:56:43 2020        
(r361038)
@@ -508,6 +508,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;
@@ -552,12 +553,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;
                }
@@ -570,7 +579,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:
@@ -578,12 +587,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,
                              /*cbfcnp*/NULL,
                              dir|CAM_DEV_QFRZDIS,
                              MSG_SIMPLE_Q_TAG,
-                             req->dxferp,
+                             data_ptr,
                              req->dxfer_len,
                              req->mx_sb_len,
                              req->cmd_len,
@@ -593,6 +611,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;
                }
@@ -611,6 +630,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/12/sys/dev/iscsi_initiator/isc_subr.c
==============================================================================
--- stable/12/sys/dev/iscsi_initiator/isc_subr.c        Thu May 14 17:54:08 
2020        (r361037)
+++ stable/12/sys/dev/iscsi_initiator/isc_subr.c        Thu May 14 17:56:43 
2020        (r361038)
@@ -194,6 +194,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);
@@ -235,15 +238,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/12/sys/net80211/ieee80211_mesh.c
==============================================================================
--- stable/12/sys/net80211/ieee80211_mesh.c     Thu May 14 17:54:08 2020        
(r361037)
+++ stable/12/sys/net80211/ieee80211_mesh.c     Thu May 14 17:56:43 2020        
(r361038)
@@ -3569,16 +3569,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;
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to