svn commit: r343372 - head/sys/dev/ixl

[Eric Joyner]

Author: erj
Date: Thu Jan 24 01:08:37 2019
New Revision: 343372
URL: https://svnweb.freebsd.org/changeset/base/343372

Log:
  ixl(4): Fix handling data passed with ioctl from NVM update tool
  
  From Krzysztof:
  
  Ensure that the entire data buffer passed from the NVM update tool is copied in
  to kernel space and copied back out to user space using copyin() and copyout().
  
  PR:		234104
  Submitted by:	Krzysztof Galazka <krzysztof.galazka at intel.com>
  Reported by:	Finn <ixbug at riseup.net>
  MFC after:	5 days
  Sponsored by:	Intel Corporation
  Differential Revision:	https://reviews.freebsd.org/D18817

Modified:
  head/sys/dev/ixl/ixl_pf_main.c

Modified: head/sys/dev/ixl/ixl_pf_main.c
==============================================================================
--- head/sys/dev/ixl/ixl_pf_main.c	Thu Jan 24 01:04:23 2019	(r343371)
+++ head/sys/dev/ixl/ixl_pf_main.c	Thu Jan 24 01:08:37 2019	(r343372)
@@ -3663,23 +3663,34 @@ ixl_handle_nvmupd_cmd(struct ixl_pf *pf, struct ifdrv 
 	struct i40e_nvm_access *nvma;
 	device_t dev = pf->dev;
 	enum i40e_status_code status = 0;
-	int perrno;
+	size_t nvma_size, ifd_len, exp_len;
+	int err, perrno;
 
 	DEBUGFUNC("ixl_handle_nvmupd_cmd");
 
 	/* Sanity checks */
-	if (ifd->ifd_len < sizeof(struct i40e_nvm_access) ||
+	nvma_size = sizeof(struct i40e_nvm_access);
+	ifd_len = ifd->ifd_len;
+
+	if (ifd_len < nvma_size ||
 	    ifd->ifd_data == NULL) {
 		device_printf(dev, "%s: incorrect ifdrv length or data pointer\n",
 		    __func__);
 		device_printf(dev, "%s: ifdrv length: %zu, sizeof(struct i40e_nvm_access): %zu\n",
-		    __func__, ifd->ifd_len, sizeof(struct i40e_nvm_access));
+		    __func__, ifd_len, nvma_size);
 		device_printf(dev, "%s: data pointer: %p\n", __func__,
 		    ifd->ifd_data);
 		return (EINVAL);
 	}
 
-	nvma = (struct i40e_nvm_access *)ifd->ifd_data;
+	nvma = malloc(ifd_len, M_DEVBUF, M_WAITOK);
+	err = copyin(ifd->ifd_data, nvma, ifd_len);
+	if (err) {
+		device_printf(dev, "%s: Cannot get request from user space\n",
+		    __func__);
+		free(nvma, M_DEVBUF);
+		return (err);
+	}
 
 	if (pf->dbg_mask & IXL_DBG_NVMUPD)
 		ixl_print_nvm_cmd(dev, nvma);
@@ -3693,13 +3704,49 @@ ixl_handle_nvmupd_cmd(struct ixl_pf *pf, struct ifdrv 
 		}
 	}
 
-	if (!(pf->state & IXL_PF_STATE_ADAPTER_RESETTING)) {
-		// TODO: Might need a different lock here
-		// IXL_PF_LOCK(pf);
-		status = i40e_nvmupd_command(hw, nvma, nvma->data, &perrno);
-		// IXL_PF_UNLOCK(pf);
-	} else {
-		perrno = -EBUSY;
+	if (pf->state & IXL_PF_STATE_ADAPTER_RESETTING) {
+		free(nvma, M_DEVBUF);
+		return (-EBUSY);
+	}
+
+	if (nvma->data_size < 1 || nvma->data_size > 4096) {
+		device_printf(dev, "%s: invalid request, data size not in supported range\n",
+		    __func__);
+		free(nvma, M_DEVBUF);
+		return (EINVAL);
+	}
+
+	/*
+	 * Older versions of the NVM update tool don't set ifd_len to the size
+	 * of the entire buffer passed to the ioctl. Check the data_size field
+	 * in the contained i40e_nvm_access struct and ensure everything is
+	 * copied in from userspace.
+	 */
+	exp_len = nvma_size + nvma->data_size - 1; /* One byte is kept in struct */
+
+	if (ifd_len < exp_len) {
+		ifd_len = exp_len;
+		nvma = realloc(nvma, ifd_len, M_DEVBUF, M_WAITOK);
+		err = copyin(ifd->ifd_data, nvma, ifd_len);
+		if (err) {
+			device_printf(dev, "%s: Cannot get request from user space\n",
+					__func__);
+			free(nvma, M_DEVBUF);
+			return (err);
+		}
+	}
+
+	// TODO: Might need a different lock here
+	// IXL_PF_LOCK(pf);
+	status = i40e_nvmupd_command(hw, nvma, nvma->data, &perrno);
+	// IXL_PF_UNLOCK(pf);
+
+	err = copyout(nvma, ifd->ifd_data, ifd_len);
+	free(nvma, M_DEVBUF);
+	if (err) {
+		device_printf(dev, "%s: Cannot return data to user space\n",
+				__func__);
+		return (err);
 	}
 
 	/* Let the nvmupdate report errors, show them only when debug is enabled */