[src/jdolecek-ncqfixes]: src/sys/dev/ata fix use-after-free in wd(4) dump, de...

[jdolecek <jdolecek%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/7db6c7bb3860
branches:  jdolecek-ncqfixes
changeset: 850582:7db6c7bb3860
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Sat Sep 22 16:14:25 2018 +0000

description:
fix use-after-free in wd(4) dump, detected by switch to the pool

change code in wd_dumpblocks() to use it's own non-pool ata_xfer,
which skips the deallocation step and thus keeps the contents when the I/O
is finished

diffstat:

 sys/dev/ata/TODO.ncq |   2 --
 sys/dev/ata/ata.c    |  11 ++++++-----
 sys/dev/ata/atavar.h |   3 ++-
 sys/dev/ata/wd.c     |  15 ++++++---------
 sys/dev/ata/wdvar.h  |   4 +++-
 5 files changed, 17 insertions(+), 18 deletions(-)

diffs (143 lines):

diff -r 4e7fae7cb16c -r 7db6c7bb3860 sys/dev/ata/TODO.ncq
--- a/sys/dev/ata/TODO.ncq      Sat Sep 22 12:20:31 2018 +0000
+++ b/sys/dev/ata/TODO.ncq      Sat Sep 22 16:14:25 2018 +0000
@@ -1,8 +1,6 @@
 jdolecek-ncqfixes goals:
 - add to wd(4) a callout to restart buf queue processing when ata_get_xfer()
   call fails and remove ata_channel_start()
-- change wd(4) dump code to use preallocated or on-stack ata_xfer to not rely
-  on pool having memory
 - re-fix QEMU ahci(4) bug workaround (no READ LOG EXT support) - now it
   triggers KASSERT()
 - fix ahci(4) error handling under paralles - invalid bio via WD_CHAOS_MONKEY
diff -r 4e7fae7cb16c -r 7db6c7bb3860 sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Sat Sep 22 12:20:31 2018 +0000
+++ b/sys/dev/ata/ata.c Sat Sep 22 16:14:25 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata.c,v 1.141.6.8 2018/09/22 12:20:31 jdolecek Exp $   */
+/*     $NetBSD: ata.c,v 1.141.6.9 2018/09/22 16:14:25 jdolecek Exp $   */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.  All rights reserved.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.141.6.8 2018/09/22 12:20:31 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.141.6.9 2018/09/22 16:14:25 jdolecek Exp $");
 
 #include "opt_ata.h"
 
@@ -1350,7 +1350,7 @@
 
        ata_channel_lock(chp);
 
-       if (xfer->c_flags & (C_WAITACT|C_WAITTIMO)) {
+       if (__predict_false(xfer->c_flags & (C_WAITACT|C_WAITTIMO))) {
                /* Someone is waiting for this xfer, so we can't free now */
                xfer->c_flags |= C_FREE;
                cv_broadcast(&chq->c_active);
@@ -1360,7 +1360,7 @@
 
        /* XXX move PIOBM and free_gw to deactivate? */
 #if NATA_PIOBM         /* XXX wdc dependent code */
-       if (xfer->c_flags & C_PIOBM) {
+       if (__predict_false(xfer->c_flags & C_PIOBM)) {
                struct wdc_softc *wdc = CHAN_TO_WDC(chp);
 
                /* finish the busmastering PIO */
@@ -1375,7 +1375,8 @@
  
        ata_channel_unlock(chp);
 
-       pool_put(&ata_xfer_pool, xfer);
+       if (__predict_true(!ISSET(xfer->c_flags, C_PRIVATE_ALLOC)))
+               pool_put(&ata_xfer_pool, xfer);
 }
 
 void
diff -r 4e7fae7cb16c -r 7db6c7bb3860 sys/dev/ata/atavar.h
--- a/sys/dev/ata/atavar.h      Sat Sep 22 12:20:31 2018 +0000
+++ b/sys/dev/ata/atavar.h      Sat Sep 22 16:14:25 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atavar.h,v 1.99.2.5 2018/09/22 09:22:59 jdolecek Exp $ */
+/*     $NetBSD: atavar.h,v 1.99.2.6 2018/09/22 16:14:25 jdolecek Exp $ */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -198,6 +198,7 @@
 #define C_WAITTIMO     0x0400          /* race vs. timeout */
 #define C_CHAOS                0x0800          /* forced error xfer */
 #define C_RECOVERED    0x1000          /* error recovered, no need for reset */
+#define C_PRIVATE_ALLOC        0x2000          /* private alloc, skip pool_put() */
 
 /* reasons for c_kill_xfer() */
 #define KILL_GONE 1            /* device is gone while xfer was active */
diff -r 4e7fae7cb16c -r 7db6c7bb3860 sys/dev/ata/wd.c
--- a/sys/dev/ata/wd.c  Sat Sep 22 12:20:31 2018 +0000
+++ b/sys/dev/ata/wd.c  Sat Sep 22 16:14:25 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: wd.c,v 1.441.2.4 2018/09/22 09:22:59 jdolecek Exp $ */
+/*     $NetBSD: wd.c,v 1.441.2.5 2018/09/22 16:14:25 jdolecek Exp $ */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.  All rights reserved.
@@ -54,7 +54,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: wd.c,v 1.441.2.4 2018/09/22 09:22:59 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: wd.c,v 1.441.2.5 2018/09/22 16:14:25 jdolecek Exp $");
 
 #include "opt_ata.h"
 #include "opt_wd.h"
@@ -1458,7 +1458,7 @@
        struct wd_softc *wd = device_private(dev);
        struct dk_softc *dksc = &wd->sc_dksc;
        struct disk_geom *dg = &dksc->sc_dkdev.dk_geom;
-       struct ata_xfer *xfer;
+       struct ata_xfer *xfer = &wd->dump_xfer;
        int err;
 
        /* Recalibrate, if first dump transfer. */
@@ -1469,11 +1469,8 @@
                wd->drvp->state = RESET;
        }
 
-       xfer = ata_get_xfer(wd->drvp->chnl_softc, false);
-       if (xfer == NULL) {
-               printf("%s: no xfer\n", __func__);
-               return EAGAIN;
-       }
+       memset(xfer, 0, sizeof(*xfer));
+       xfer->c_flags |= C_PRIVATE_ALLOC;
 
        xfer->c_bio.blkno = blkno;
        xfer->c_bio.flags = ATA_POLL;
@@ -1519,7 +1516,7 @@
                err = 0;
                break;
        default:
-               panic("wddump: unknown error type %d", err);
+               panic("wddump: unknown error type %x", err);
        }
 
        if (err != 0) {
diff -r 4e7fae7cb16c -r 7db6c7bb3860 sys/dev/ata/wdvar.h
--- a/sys/dev/ata/wdvar.h       Sat Sep 22 12:20:31 2018 +0000
+++ b/sys/dev/ata/wdvar.h       Sat Sep 22 16:14:25 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: wdvar.h,v 1.46.6.2 2018/09/22 09:22:59 jdolecek Exp $  */
+/*     $NetBSD: wdvar.h,v 1.46.6.3 2018/09/22 16:14:25 jdolecek Exp $  */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -71,6 +71,8 @@
        SLIST_HEAD(, ata_xfer) sc_requeue_list;
        struct callout sc_requeue_callout;      /* requeue callout handle */
 
+       struct ata_xfer dump_xfer;
+
        /* Sysctl nodes specific for the disk */
        struct sysctllog *nodelog;
        bool drv_ncq;