Commit a9782df2 authored by Dennis Dalessandro's avatar Dennis Dalessandro Committed by Ben Hutchings

IB/ipath: Fix potential buffer overrun in sending diag packet routine

commit a2cb0eb8 upstream.

Guard against a potential buffer overrun.  The size to read from the
user is passed in, and due to the padding that needs to be taken into
account, as well as the place holder for the ICRC it is possible to
overflow the 32bit value which would cause more data to be copied from
user space than is allocated in the buffer.
Reported-by: default avatarNico Golde <nico@ngolde.de>
Reported-by: default avatarFabian Yamaguchi <fabs@goesec.de>
Reviewed-by: default avatarMike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: default avatarDennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: default avatarRoland Dreier <roland@purestorage.com>
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent a7bbda74
...@@ -326,7 +326,7 @@ static ssize_t ipath_diagpkt_write(struct file *fp, ...@@ -326,7 +326,7 @@ static ssize_t ipath_diagpkt_write(struct file *fp,
size_t count, loff_t *off) size_t count, loff_t *off)
{ {
u32 __iomem *piobuf; u32 __iomem *piobuf;
u32 plen, clen, pbufn; u32 plen, pbufn, maxlen_reserve;
struct ipath_diag_pkt odp; struct ipath_diag_pkt odp;
struct ipath_diag_xpkt dp; struct ipath_diag_xpkt dp;
u32 *tmpbuf = NULL; u32 *tmpbuf = NULL;
...@@ -335,51 +335,29 @@ static ssize_t ipath_diagpkt_write(struct file *fp, ...@@ -335,51 +335,29 @@ static ssize_t ipath_diagpkt_write(struct file *fp,
u64 val; u64 val;
u32 l_state, lt_state; /* LinkState, LinkTrainingState */ u32 l_state, lt_state; /* LinkState, LinkTrainingState */
if (count < sizeof(odp)) {
ret = -EINVAL;
goto bail;
}
if (count == sizeof(dp)) { if (count == sizeof(dp)) {
if (copy_from_user(&dp, data, sizeof(dp))) { if (copy_from_user(&dp, data, sizeof(dp))) {
ret = -EFAULT; ret = -EFAULT;
goto bail; goto bail;
} }
} else if (copy_from_user(&odp, data, sizeof(odp))) { } else if (count == sizeof(odp)) {
ret = -EFAULT; if (copy_from_user(&odp, data, sizeof(odp))) {
ret = -EFAULT;
goto bail;
}
} else {
ret = -EINVAL;
goto bail; goto bail;
} }
/*
* Due to padding/alignment issues (lessened with new struct)
* the old and new structs are the same length. We need to
* disambiguate them, which we can do because odp.len has never
* been less than the total of LRH+BTH+DETH so far, while
* dp.unit (same offset) unit is unlikely to get that high.
* Similarly, dp.data, the pointer to user at the same offset
* as odp.unit, is almost certainly at least one (512byte)page
* "above" NULL. The if-block below can be omitted if compatibility
* between a new driver and older diagnostic code is unimportant.
* compatibility the other direction (new diags, old driver) is
* handled in the diagnostic code, with a warning.
*/
if (dp.unit >= 20 && dp.data < 512) {
/* very probable version mismatch. Fix it up */
memcpy(&odp, &dp, sizeof(odp));
/* We got a legacy dp, copy elements to dp */
dp.unit = odp.unit;
dp.data = odp.data;
dp.len = odp.len;
dp.pbc_wd = 0; /* Indicate we need to compute PBC wd */
}
/* send count must be an exact number of dwords */ /* send count must be an exact number of dwords */
if (dp.len & 3) { if (dp.len & 3) {
ret = -EINVAL; ret = -EINVAL;
goto bail; goto bail;
} }
clen = dp.len >> 2; plen = dp.len >> 2;
dd = ipath_lookup(dp.unit); dd = ipath_lookup(dp.unit);
if (!dd || !(dd->ipath_flags & IPATH_PRESENT) || if (!dd || !(dd->ipath_flags & IPATH_PRESENT) ||
...@@ -422,16 +400,22 @@ static ssize_t ipath_diagpkt_write(struct file *fp, ...@@ -422,16 +400,22 @@ static ssize_t ipath_diagpkt_write(struct file *fp,
goto bail; goto bail;
} }
/* need total length before first word written */ /*
/* +1 word is for the qword padding */ * need total length before first word written, plus 2 Dwords. One Dword
plen = sizeof(u32) + dp.len; * is for padding so we get the full user data when not aligned on
* a word boundary. The other Dword is to make sure we have room for the
if ((plen + 4) > dd->ipath_ibmaxlen) { * ICRC which gets tacked on later.
*/
maxlen_reserve = 2 * sizeof(u32);
if (dp.len > dd->ipath_ibmaxlen - maxlen_reserve) {
ipath_dbg("Pkt len 0x%x > ibmaxlen %x\n", ipath_dbg("Pkt len 0x%x > ibmaxlen %x\n",
plen - 4, dd->ipath_ibmaxlen); dp.len, dd->ipath_ibmaxlen);
ret = -EINVAL; ret = -EINVAL;
goto bail; /* before writing pbc */ goto bail;
} }
plen = sizeof(u32) + dp.len;
tmpbuf = vmalloc(plen); tmpbuf = vmalloc(plen);
if (!tmpbuf) { if (!tmpbuf) {
dev_info(&dd->pcidev->dev, "Unable to allocate tmp buffer, " dev_info(&dd->pcidev->dev, "Unable to allocate tmp buffer, "
...@@ -473,11 +457,11 @@ static ssize_t ipath_diagpkt_write(struct file *fp, ...@@ -473,11 +457,11 @@ static ssize_t ipath_diagpkt_write(struct file *fp,
*/ */
if (dd->ipath_flags & IPATH_PIO_FLUSH_WC) { if (dd->ipath_flags & IPATH_PIO_FLUSH_WC) {
ipath_flush_wc(); ipath_flush_wc();
__iowrite32_copy(piobuf + 2, tmpbuf, clen - 1); __iowrite32_copy(piobuf + 2, tmpbuf, plen - 1);
ipath_flush_wc(); ipath_flush_wc();
__raw_writel(tmpbuf[clen - 1], piobuf + clen + 1); __raw_writel(tmpbuf[plen - 1], piobuf + plen + 1);
} else } else
__iowrite32_copy(piobuf + 2, tmpbuf, clen); __iowrite32_copy(piobuf + 2, tmpbuf, plen);
ipath_flush_wc(); ipath_flush_wc();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment