Date: Mon, 6 Mar 2017 06:32:14 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r314749 - stable/10/sys/cam/ctl Message-ID: <201703060632.v266WEYO097545@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Mon Mar 6 06:32:14 2017 New Revision: 314749 URL: https://svnweb.freebsd.org/changeset/base/314749 Log: MFC r314246: Improve CAM target frontend reference counting. Before this change it was possible to trigger some use-after-free panics by disabling LUNs/ports under heavy load. Modified: stable/10/sys/cam/ctl/scsi_ctl.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/cam/ctl/scsi_ctl.c ============================================================================== --- stable/10/sys/cam/ctl/scsi_ctl.c Mon Mar 6 06:31:42 2017 (r314748) +++ stable/10/sys/cam/ctl/scsi_ctl.c Mon Mar 6 06:32:14 2017 (r314749) @@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$"); #include <sys/sysctl.h> #include <sys/types.h> #include <sys/systm.h> +#include <sys/taskqueue.h> #include <machine/bus.h> #include <cam/cam.h> @@ -103,15 +104,11 @@ struct ctlfe_lun_softc { struct ctlfe_softc *parent_softc; struct cam_periph *periph; ctlfe_lun_flags flags; - uint64_t ccbs_alloced; - uint64_t ccbs_freed; - uint64_t ctios_sent; - uint64_t ctios_returned; - uint64_t atios_alloced; - uint64_t atios_freed; - uint64_t inots_alloced; - uint64_t inots_freed; - /* bus_dma_tag_t dma_tag; */ + int ctios_sent; /* Number of active CTIOs */ + int refcount; /* Number of active xpt_action() */ + int atios_alloced; /* Number of ATIOs not freed */ + int inots_alloced; /* Number of INOTs not freed */ + struct task refdrain_task; TAILQ_HEAD(, ccb_hdr) work_queue; STAILQ_ENTRY(ctlfe_lun_softc) links; }; @@ -683,18 +680,14 @@ ctlfecleanup(struct cam_periph *periph) softc = (struct ctlfe_lun_softc *)periph->softc; - KASSERT(softc->ccbs_freed == softc->ccbs_alloced, ("%s: " - "ccbs_freed %ju != ccbs_alloced %ju", __func__, - softc->ccbs_freed, softc->ccbs_alloced)); - KASSERT(softc->ctios_returned == softc->ctios_sent, ("%s: " - "ctios_returned %ju != ctios_sent %ju", __func__, - softc->ctios_returned, softc->ctios_sent)); - KASSERT(softc->atios_freed == softc->atios_alloced, ("%s: " - "atios_freed %ju != atios_alloced %ju", __func__, - softc->atios_freed, softc->atios_alloced)); - KASSERT(softc->inots_freed == softc->inots_alloced, ("%s: " - "inots_freed %ju != inots_alloced %ju", __func__, - softc->inots_freed, softc->inots_alloced)); + KASSERT(softc->ctios_sent == 0, ("%s: ctios_sent %d != 0", + __func__, softc->ctios_sent)); + KASSERT(softc->refcount == 0, ("%s: refcount %d != 0", + __func__, softc->refcount)); + KASSERT(softc->atios_alloced == 0, ("%s: atios_alloced %d != 0", + __func__, softc->atios_alloced)); + KASSERT(softc->inots_alloced == 0, ("%s: inots_alloced %d != 0", + __func__, softc->inots_alloced)); free(softc, M_CTLFE); } @@ -810,12 +803,10 @@ ctlfestart(struct cam_periph *periph, un uint8_t scsi_status; softc = (struct ctlfe_lun_softc *)periph->softc; - softc->ccbs_alloced++; next: ccb_h = TAILQ_FIRST(&softc->work_queue); if (ccb_h == NULL) { - softc->ccbs_freed++; xpt_release_ccb(start_ccb); return; } @@ -937,16 +928,32 @@ next: io->io_hdr.flags &= ~(CTL_FLAG_DMA_QUEUED | CTL_FLAG_STATUS_QUEUED); softc->ctios_sent++; - + softc->refcount++; cam_periph_unlock(periph); xpt_action(start_ccb); cam_periph_lock(periph); + softc->refcount--; /* * If we still have work to do, ask for another CCB. */ if (!TAILQ_EMPTY(&softc->work_queue)) - xpt_schedule(periph, /*priority*/ 1); + xpt_schedule(periph, CAM_PRIORITY_NORMAL); +} + +static void +ctlfe_drain(void *context, int pending) +{ + struct cam_periph *periph = context; + struct ctlfe_lun_softc *softc = periph->softc; + + cam_periph_lock(periph); + while (softc->refcount != 0) { + cam_periph_sleep(periph, &softc->refcount, PRIBIO, + "ctlfe_drain", 1); + } + cam_periph_unlock(periph); + cam_periph_release(periph); } static void @@ -961,13 +968,13 @@ ctlfe_free_ccb(struct cam_periph *periph switch (ccb->ccb_h.func_code) { case XPT_ACCEPT_TARGET_IO: - softc->atios_freed++; + softc->atios_alloced--; cmd_info = PRIV_INFO(io); free(cmd_info, M_CTLFE); break; case XPT_IMMEDIATE_NOTIFY: case XPT_NOTIFY_ACKNOWLEDGE: - softc->inots_freed++; + softc->inots_alloced--; break; default: break; @@ -976,21 +983,24 @@ ctlfe_free_ccb(struct cam_periph *periph ctl_free_io(io); free(ccb, M_CTLFE); - KASSERT(softc->atios_freed <= softc->atios_alloced, ("%s: " - "atios_freed %ju > atios_alloced %ju", __func__, - softc->atios_freed, softc->atios_alloced)); - KASSERT(softc->inots_freed <= softc->inots_alloced, ("%s: " - "inots_freed %ju > inots_alloced %ju", __func__, - softc->inots_freed, softc->inots_alloced)); + KASSERT(softc->atios_alloced >= 0, ("%s: atios_alloced %d < 0", + __func__, softc->atios_alloced)); + KASSERT(softc->inots_alloced >= 0, ("%s: inots_alloced %d < 0", + __func__, softc->inots_alloced)); /* * If we have received all of our CCBs, we can release our * reference on the peripheral driver. It will probably go away * now. */ - if ((softc->atios_freed == softc->atios_alloced) - && (softc->inots_freed == softc->inots_alloced)) { - cam_periph_release_locked(periph); + if (softc->atios_alloced == 0 && softc->inots_alloced == 0) { + if (softc->refcount == 0) { + cam_periph_release_locked(periph); + } else { + TASK_INIT(&softc->refdrain_task, 0, ctlfe_drain, periph); + taskqueue_enqueue(taskqueue_thread, + &softc->refdrain_task); + } } } @@ -1216,7 +1226,7 @@ ctlfedone(struct cam_periph *periph, uni atio = (struct ccb_accept_tio *)done_ccb->ccb_h.ccb_atio; io = (union ctl_io *)atio->ccb_h.io_ptr; - softc->ctios_returned++; + softc->ctios_sent--; #ifdef CTLFEDEBUG printf("%s: got XPT_CONT_TARGET_IO tag %#x flags %#x\n", __func__, atio->tag_id, done_ccb->ccb_h.flags); @@ -1252,11 +1262,10 @@ ctlfedone(struct cam_periph *periph, uni io->scsiio.ext_data_filled = srr_off; io->scsiio.io_hdr.status = CTL_STATUS_NONE; io->io_hdr.flags |= CTL_FLAG_DMA_QUEUED; - softc->ccbs_freed++; xpt_release_ccb(done_ccb); TAILQ_INSERT_HEAD(&softc->work_queue, &atio->ccb_h, periph_links.tqe); - xpt_schedule(periph, /*priority*/ 1); + xpt_schedule(periph, CAM_PRIORITY_NORMAL); break; } @@ -1267,7 +1276,6 @@ ctlfedone(struct cam_periph *periph, uni * should work. */ if (srr && (io->io_hdr.flags & CTL_FLAG_DMA_INPROG) == 0) { - softc->ccbs_freed++; xpt_release_ccb(done_ccb); if (ctlfe_adjust_cdb(atio, srr_off) == 0) { done_ccb = (union ccb *)atio; @@ -1296,7 +1304,6 @@ ctlfedone(struct cam_periph *periph, uni xpt_action(done_ccb); } - softc->ccbs_freed++; xpt_release_ccb(done_ccb); ctlfe_requeue_ccb(periph, (union ccb *)atio, /* unlock */1); @@ -1395,7 +1402,6 @@ ctlfedone(struct cam_periph *periph, uni * Release the CTIO. The ATIO will be sent back * down to the SIM once we send status. */ - softc->ccbs_freed++; xpt_release_ccb(done_ccb); mtx_unlock(mtx); @@ -1928,16 +1934,8 @@ ctlfe_dump_queue(struct ctlfe_lun_softc } } - xpt_print(periph->path, "%d requests total waiting for CCBs\n", - num_items); - xpt_print(periph->path, "%ju CCBs outstanding (%ju allocated, %ju " - "freed)\n", (uintmax_t)(softc->ccbs_alloced - - softc->ccbs_freed), (uintmax_t)softc->ccbs_alloced, - (uintmax_t)softc->ccbs_freed); - xpt_print(periph->path, "%ju CTIOs outstanding (%ju sent, %ju " - "returned\n", (uintmax_t)(softc->ctios_sent - - softc->ctios_returned), softc->ctios_sent, - softc->ctios_returned); + xpt_print(periph->path, "%d requests waiting for CCBs\n", num_items); + xpt_print(periph->path, "%d CTIOs outstanding\n", softc->ctios_sent); } /* @@ -1965,7 +1963,7 @@ ctlfe_datamove(union ctl_io *io) io->io_hdr.flags |= CTL_FLAG_STATUS_QUEUED; TAILQ_INSERT_TAIL(&softc->work_queue, &ccb->ccb_h, periph_links.tqe); - xpt_schedule(periph, /*priority*/ 1); + xpt_schedule(periph, CAM_PRIORITY_NORMAL); cam_periph_unlock(periph); } @@ -2018,7 +2016,7 @@ ctlfe_done(union ctl_io *io) io->io_hdr.flags |= CTL_FLAG_STATUS_QUEUED; TAILQ_INSERT_TAIL(&softc->work_queue, &ccb->ccb_h, periph_links.tqe); - xpt_schedule(periph, /*priority*/ 1); + xpt_schedule(periph, CAM_PRIORITY_NORMAL); } cam_periph_unlock(periph);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703060632.v266WEYO097545>