Hi all,

ncr53c9x_rdfifo() in sys/dev/ic/ncr53c9x.c drains the hardware FIFO into
sc->sc_imess, which is allocated as NCR_MAX_MSG_LEN + 1 (9) bytes. The FIFO
byte count from NCR_FFLAG can be up to 31, or up to 63 on FAS366 variants
(doubled plus a possible shuttle byte). There is no bounds check against
the buffer size before the write loop, which can result in a heap buffer
overflow that corrupts adjacent kernel memory. The existing XXX comment at
line 1251 notes the size mismatch.

The patch below replaces the XXX with a computed remaining-space bound and
clamps n in both the FAS366 and non-FAS366 paths. For the FAS366 shuttle
byte, the FIFO register is still read (to keep hardware state consistent)
but the store is skipped if the buffer is full.

Best regards,
Stanislav Fort
Aisle Research

---

Draft patch:

diff --git a/sys/dev/ic/ncr53c9x.c b/sys/dev/ic/ncr53c9x.c
--- a/sys/dev/ic/ncr53c9x.c
+++ b/sys/dev/ic/ncr53c9x.c
@@ -1232,7 +1232,7 @@ static int
 ncr53c9x_rdfifo(struct ncr53c9x_softc *sc, int how)
 {
- int i, n;
+ int i, n, remaining;
  u_char *buf;

  switch(how) {
@@ -1249,14 +1249,16 @@ ncr53c9x_rdfifo(struct ncr53c9x_softc *sc, int how)
  break;
  }

- /*
- * XXX buffer (sc_imess) size for message
- */
+ remaining = NCR_MAX_MSG_LEN + 1 - (int)sc->sc_imlen;
+ if (remaining < 0)
+ remaining = 0;

  n = NCR_READ_REG(sc, NCR_FFLAG) & NCRFIFO_FF;

  if (sc->sc_rev == NCR_VARIANT_FAS366) {
  n *= 2;
+ if (n > remaining)
+ n = remaining;

  for (i = 0; i < n; i++)
  buf[i] = NCR_READ_REG(sc, NCR_FIFO);
@@ -1264,7 +1266,10 @@ ncr53c9x_rdfifo(struct ncr53c9x_softc *sc, int how)
  if (sc->sc_espstat2 & FAS_STAT2_ISHUTTLE) {

  NCR_WRITE_REG(sc, NCR_FIFO, 0);
- buf[i++] = NCR_READ_REG(sc, NCR_FIFO);
+ if (i < remaining)
+ buf[i++] = NCR_READ_REG(sc, NCR_FIFO);
+ else
+ (void)NCR_READ_REG(sc, NCR_FIFO);

  NCR_READ_REG(sc, NCR_FIFO);

@@ -1272,6 +1277,8 @@ ncr53c9x_rdfifo(struct ncr53c9x_softc *sc, int how)
  }
  } else {
+ if (n > remaining)
+ n = remaining;
  for (i = 0; i < n; i++)
  buf[i] = NCR_READ_REG(sc, NCR_FIFO);
  }