hi, looks like we're missing some device_unref's that need to be there after we call device_lookup or disk_lookup or numerous macros that expand to either of those.
it would be nice if someone with working hibernate and ahci could test the diff (on either i386 or amd64). i'm also not 100% sure if wddump should do unreferencing since machine is going down anyways. please note that the only place left to fix is the zs(4) driver and it's a wee bit more involved. ok? diff --git sys/arch/amd64/amd64/hibernate_machdep.c sys/arch/amd64/amd64/hibernate_machdep.c index 1c0487e..517eddd 100644 --- sys/arch/amd64/amd64/hibernate_machdep.c +++ sys/arch/amd64/amd64/hibernate_machdep.c @@ -92,8 +92,10 @@ get_hibernate_io_function(void) dv = disk_lookup(&sd_cd, DISKUNIT(swdevt[0].sw_dev)); if (dv && dv->dv_parent && dv->dv_parent->dv_parent && strcmp(dv->dv_parent->dv_parent->dv_cfdata->cf_driver->cd_name, - "ahci") == 0) + "ahci") == 0) { + device_unref(dv); return ahci_hibernate_io; + } } #endif return NULL; diff --git sys/arch/i386/i386/hibernate_machdep.c sys/arch/i386/i386/hibernate_machdep.c index 7dc15e3..04eda8b 100644 --- sys/arch/i386/i386/hibernate_machdep.c +++ sys/arch/i386/i386/hibernate_machdep.c @@ -90,8 +90,10 @@ get_hibernate_io_function(void) dv = disk_lookup(&sd_cd, DISKUNIT(swdevt[0].sw_dev)); if (dv && dv->dv_parent && dv->dv_parent->dv_parent && strcmp(dv->dv_parent->dv_parent->dv_cfdata->cf_driver->cd_name, - "ahci") == 0) + "ahci") == 0) { + device_unref(dv); return ahci_hibernate_io; + } } #endif return NULL; diff --git sys/dev/ata/ata_wdc.c sys/dev/ata/ata_wdc.c index d31fe09..40e6c98 100644 --- sys/dev/ata/ata_wdc.c +++ sys/dev/ata/ata_wdc.c @@ -143,6 +143,7 @@ wd_hibernate_io(dev_t dev, daddr_t blkno, vaddr_t addr, size_t size, int op, voi /* The ca_activate function for the parent controller */ (cfd->cf_attach->ca_activate)(dv, DVACT_RESUME); + device_unref(&real_wd->sc_dev); return (0); } @@ -180,6 +181,7 @@ wd_hibernate_io(dev_t dev, daddr_t blkno, vaddr_t addr, size_t size, int op, voi xfer->c_intr = wdc_ata_bio_intr; xfer->c_kill_xfer = wdc_ata_bio_kill_xfer; wdc_exec_xfer(chp, xfer); + device_unref(&real_wd->sc_dev); return (ata_bio->flags & ATA_ITSDONE) ? 0 : EIO; } #endif /* HIBERNATE */ diff --git sys/dev/ata/wd.c sys/dev/ata/wd.c index 7edd174..45c8c7a 100644 --- sys/dev/ata/wd.c +++ sys/dev/ata/wd.c @@ -626,8 +626,10 @@ wdopen(dev_t dev, int flag, int fmt, struct proc *p) if (wd == NULL) return ENXIO; chnl = (struct channel_softc *)(wd->drvp->chnl_softc); - if (chnl->dying) + if (chnl->dying) { + device_unref(&wd->sc_dev); return (ENXIO); + } /* * If this is the first open of this device, add a reference @@ -935,19 +937,26 @@ wddump(dev_t dev, daddr64_t blkno, caddr_t va, size_t size) part = DISKPART(dev); /* Make sure it was initialized. */ - if (wd->drvp->state < READY) + if (wd->drvp->state < READY) { + device_unref(&wd->sc_dev); return ENXIO; + } /* Convert to disk sectors. Request must be a multiple of size. */ lp = wd->sc_dk.dk_label; - if ((size % lp->d_secsize) != 0) + if ((size % lp->d_secsize) != 0) { + device_unref(&wd->sc_dev); return EFAULT; + } nblks = size / lp->d_secsize; blkno = blkno / (lp->d_secsize / DEV_BSIZE); /* Check transfer bounds against partition size. */ - if ((blkno < 0) || ((blkno + nblks) > DL_GETPSIZE(&lp->d_partitions[part]))) + if ((blkno < 0) || ((blkno + nblks) > + DL_GETPSIZE(&lp->d_partitions[part]))) { + device_unref(&wd->sc_dev); return EINVAL; + } /* Offset block number to start of partition. */ blkno += DL_GETPOFFSET(&lp->d_partitions[part]); @@ -1009,6 +1018,7 @@ wddump(dev_t dev, daddr64_t blkno, caddr_t va, size_t size) } if (err != 0) { printf("\n"); + device_unref(&wd->sc_dev); return err; } #else /* WD_DUMP_NOT_TRUSTED */ @@ -1025,6 +1035,7 @@ wddump(dev_t dev, daddr64_t blkno, caddr_t va, size_t size) } wddoingadump = 0; + device_unref(&wd->sc_dev); return 0; } diff --git sys/dev/flash.c sys/dev/flash.c index 8f5dd8d..af67532 100644 --- sys/dev/flash.c +++ sys/dev/flash.c @@ -1019,6 +1019,8 @@ flashminphys(struct buf *bp) if (bp->b_bcount > sc->sc_flashdev->pagesize) bp->b_bcount = sc->sc_flashdev->pagesize; + + device_unref(&sc->sc_dev); } int diff --git sys/dev/gpio/gpio.c sys/dev/gpio/gpio.c index 0bca78d..31f8ede 100644 --- sys/dev/gpio/gpio.c +++ sys/dev/gpio/gpio.c @@ -254,10 +254,14 @@ gpioopen(dev_t dev, int flag, int mode, struct proc *p) if (sc == NULL) return (ENXIO); - if (sc->sc_opened) + if (sc->sc_opened) { + device_unref(&sc->sc_dev); return (EBUSY); + } sc->sc_opened = 1; + device_unref(&sc->sc_dev); + return (0); } @@ -267,7 +271,10 @@ gpioclose(dev_t dev, int flag, int mode, struct proc *p) struct gpio_softc *sc; sc = (struct gpio_softc *)device_lookup(&gpio_cd, minor(dev)); + if (sc == NULL) + return (ENXIO); sc->sc_opened = 0; + device_unref(&sc->sc_dev); return (0); } @@ -299,6 +306,8 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) int pin, value, flags, npins, found; sc = (struct gpio_softc *)device_lookup(&gpio_cd, minor(dev)); + if (sc == NULL) + return (ENXIO); gc = sc->sc_gc; switch (cmd) { @@ -318,47 +327,65 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) if (op->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, op->gp_name); - if (pin == -1) + if (pin == -1) { + device_unref(&sc->sc_dev); return (EINVAL); + } } else pin = op->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) + if (pin < 0 || pin >= sc->sc_npins) { + device_unref(&sc->sc_dev); return (EINVAL); + } if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) && - securelevel > 0) + securelevel > 0) { + device_unref(&sc->sc_dev); return (EPERM); + } /* return read value */ op->gp_value = gpiobus_pin_read(gc, pin); break; case GPIOPINWRITE: - if ((flag & FWRITE) == 0) + if ((flag & FWRITE) == 0) { + device_unref(&sc->sc_dev); return (EBADF); + } op = (struct gpio_pin_op *)data; if (op->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, op->gp_name); - if (pin == -1) + if (pin == -1) { + device_unref(&sc->sc_dev); return (EINVAL); + } } else pin = op->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) + if (pin < 0 || pin >= sc->sc_npins) { + device_unref(&sc->sc_dev); return (EINVAL); + } - if (sc->sc_pins[pin].pin_mapped) + if (sc->sc_pins[pin].pin_mapped) { + device_unref(&sc->sc_dev); return (EBUSY); + } if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) && - securelevel > 0) + securelevel > 0) { + device_unref(&sc->sc_dev); return (EPERM); + } value = op->gp_value; - if (value != GPIO_PIN_LOW && value != GPIO_PIN_HIGH) + if (value != GPIO_PIN_LOW && value != GPIO_PIN_HIGH) { + device_unref(&sc->sc_dev); return (EINVAL); + } gpiobus_pin_write(gc, pin, value); /* return old value */ @@ -367,27 +394,37 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) sc->sc_pins[pin].pin_state = value; break; case GPIOPINTOGGLE: - if ((flag & FWRITE) == 0) + if ((flag & FWRITE) == 0) { + device_unref(&sc->sc_dev); return (EBADF); + } op = (struct gpio_pin_op *)data; if (op->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, op->gp_name); - if (pin == -1) + if (pin == -1) { + device_unref(&sc->sc_dev); return (EINVAL); + } } else pin = op->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) + if (pin < 0 || pin >= sc->sc_npins) { + device_unref(&sc->sc_dev); return (EINVAL); + } - if (sc->sc_pins[pin].pin_mapped) + if (sc->sc_pins[pin].pin_mapped) { + device_unref(&sc->sc_dev); return (EBUSY); + } if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) && - securelevel > 0) + securelevel > 0) { + device_unref(&sc->sc_dev); return (EPERM); + } value = (sc->sc_pins[pin].pin_state == GPIO_PIN_LOW ? GPIO_PIN_HIGH : GPIO_PIN_LOW); @@ -398,8 +435,10 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) sc->sc_pins[pin].pin_state = value; break; case GPIOATTACH: - if (securelevel > 0) + if (securelevel > 0) { + device_unref(&sc->sc_dev); return (EPERM); + } attach = (struct gpio_attach *)data; bzero(&ga, sizeof(ga)); @@ -418,8 +457,10 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) } break; case GPIODETACH: - if (securelevel > 0) + if (securelevel > 0) { + device_unref(&sc->sc_dev); return (EPERM); + } attach = (struct gpio_attach *)data; LIST_FOREACH(gdev, &sc->sc_devs, sc_next) { @@ -434,23 +475,31 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) } break; case GPIOPINSET: - if (securelevel > 0) + if (securelevel > 0) { + device_unref(&sc->sc_dev); return (EPERM); + } set = (struct gpio_pin_set *)data; if (set->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, set->gp_name); - if (pin == -1) + if (pin == -1) { + device_unref(&sc->sc_dev); return (EINVAL); + } } else pin = set->gp_pin; - if (pin < 0 || pin >= sc->sc_npins) + if (pin < 0 || pin >= sc->sc_npins) { + device_unref(&sc->sc_dev); return (EINVAL); + } flags = set->gp_flags; /* check that the controller supports all requested flags */ - if ((flags & sc->sc_pins[pin].pin_caps) != flags) + if ((flags & sc->sc_pins[pin].pin_caps) != flags) { + device_unref(&sc->sc_dev); return (ENODEV); + } flags = set->gp_flags | GPIO_PIN_SET; set->gp_caps = sc->sc_pins[pin].pin_caps; @@ -483,23 +532,33 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) } break; case GPIOPINUNSET: - if (securelevel > 0) + if (securelevel > 0) { + device_unref(&sc->sc_dev); return (EPERM); + } set = (struct gpio_pin_set *)data; if (set->gp_name[0] != '\0') { pin = gpio_pinbyname(sc, set->gp_name); - if (pin == -1) + if (pin == -1) { + device_unref(&sc->sc_dev); return (EINVAL); + } } else pin = set->gp_pin; - - if (pin < 0 || pin >= sc->sc_npins) + + if (pin < 0 || pin >= sc->sc_npins) { + device_unref(&sc->sc_dev); return (EINVAL); - if (sc->sc_pins[pin].pin_mapped) + } + if (sc->sc_pins[pin].pin_mapped) { + device_unref(&sc->sc_dev); return (EBUSY); - if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET)) + } + if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET)) { + device_unref(&sc->sc_dev); return (EINVAL); + } LIST_FOREACH(nm, &sc->sc_names, gp_next) { if (nm->gp_pin == pin) { @@ -511,8 +570,11 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) sc->sc_pins[pin].pin_flags &= ~GPIO_PIN_SET; break; default: + device_unref(&sc->sc_dev); return (ENOTTY); } + device_unref(&sc->sc_dev); + return (0); } diff --git sys/dev/i2o/iop.c sys/dev/i2o/iop.c index 91f8da3..553d1cf 100644 --- sys/dev/i2o/iop.c +++ sys/dev/i2o/iop.c @@ -437,15 +437,19 @@ iop_config_interrupts(struct device *self) iop = (struct iop_softc *)device_lookup(&iop_cd, i); if (iop == NULL) continue; - if ((iop->sc_flags & IOP_HAVESTATUS) == 0) + if ((iop->sc_flags & IOP_HAVESTATUS) == 0) { + device_unref(&iop->sc_dv); continue; + } if (iop_status_get(iop, 1) != 0) { printf("%s: unable to retrieve status\n", sc->sc_dv.dv_xname); iop->sc_flags &= ~IOP_HAVESTATUS; + device_unref(&iop->sc_dv); continue; } niop++; + device_unref(&iop->sc_dv); } if (niop == 0) return; @@ -465,8 +469,10 @@ iop_config_interrupts(struct device *self) iop = (struct iop_softc *)device_lookup(&iop_cd, i); if (iop == NULL) continue; - if ((iop->sc_flags & IOP_HAVESTATUS) == 0) + if ((iop->sc_flags & IOP_HAVESTATUS) == 0) { + device_unref(&iop->sc_dv); continue; + } ste->orgid = iop->sc_status.orgid; ste->iopid = iop->sc_dv.dv_unit + 2; @@ -478,6 +484,8 @@ iop_config_interrupts(struct device *self) ste->inboundmsgportaddresslow = htole32(iop->sc_memaddr + IOP_REG_IFIFO); ste++; + + device_unref(&iop->sc_dv); } } @@ -868,8 +876,10 @@ iop_shutdown(void *junk) for (i = 0; i < iop_cd.cd_ndevs; i++) { if (!(sc = (struct iop_softc *)device_lookup(&iop_cd, i))) continue; - if ((sc->sc_flags & IOP_ONLINE) == 0) + if ((sc->sc_flags & IOP_ONLINE) == 0) { + device_unref(&sc->sc_dv); continue; + } iop_simple_cmd(sc, I2O_TID_IOP, I2O_EXEC_SYS_QUIESCE, IOP_ICTX, 0, 5000); @@ -882,6 +892,8 @@ iop_shutdown(void *junk) iop_simple_cmd(sc, I2O_TID_IOP, I2O_EXEC_IOP_CLEAR, IOP_ICTX, 0, 1000); } + + device_unref(&sc->sc_dv); } /* Wait. Some boards could still be flushing, stupidly enough. */ @@ -2294,19 +2306,26 @@ iopopen(dev_t dev, int flag, int mode, struct proc *p) if (!(sc = (struct iop_softc *)device_lookup(&iop_cd, minor(dev)))) return (ENXIO); - if ((sc->sc_flags & IOP_ONLINE) == 0) + if ((sc->sc_flags & IOP_ONLINE) == 0) { + device_unref(&sc->sc_dv); return (ENXIO); - if ((sc->sc_flags & IOP_OPEN) != 0) + } + if ((sc->sc_flags & IOP_OPEN) != 0) { + device_unref(&sc->sc_dv); return (EBUSY); + } sc->sc_flags |= IOP_OPEN; sc->sc_ptb = malloc(IOP_MAX_XFER * IOP_MAX_MSG_XFERS, M_DEVBUF, M_WAITOK | M_CANFAIL); if (sc->sc_ptb == NULL) { sc->sc_flags ^= IOP_OPEN; + device_unref(&sc->sc_dv); return (ENOMEM); } + device_unref(&sc->sc_dv); + return (0); } @@ -2318,6 +2337,7 @@ iopclose(dev_t dev, int flag, int mode, struct proc *p) sc = (struct iop_softc *)device_lookup(&iop_cd, minor(dev)); /* XXX */ free(sc->sc_ptb, M_DEVBUF); sc->sc_flags &= ~IOP_OPEN; + device_unref(&sc->sc_dv); return (0); } @@ -2332,10 +2352,14 @@ iopioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) return (EPERM); sc = (struct iop_softc *)device_lookup(&iop_cd, minor(dev)); /* XXX */ + if (sc == NULL) + return (ENXIO); switch (cmd) { case IOPIOCPT: - return (iop_passthrough(sc, (struct ioppt *)data)); + rv = iop_passthrough(sc, (struct ioppt *)data); + device_unref(&sc->sc_dv); + return (rv); case IOPIOCGSTATUS: iov = (struct iovec *)data; @@ -2346,6 +2370,7 @@ iopioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) iov->iov_len = i; if ((rv = iop_status_get(sc, 0)) == 0) rv = copyout(&sc->sc_status, iov->iov_base, i); + device_unref(&sc->sc_dv); return (rv); case IOPIOCGLCT: @@ -2357,6 +2382,7 @@ iopioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) #if defined(DIAGNOSTIC) || defined(I2ODEBUG) printf("%s: unknown ioctl %lx\n", sc->sc_dv.dv_xname, cmd); #endif + device_unref(&sc->sc_dv); return (ENOTTY); } @@ -2389,6 +2415,7 @@ iopioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p) } rw_exit_read(&sc->sc_conflock); + device_unref(&sc->sc_dv); return (rv); } diff --git sys/dev/ic/ahci.c sys/dev/ic/ahci.c index 5033c9c..62ff732 100644 --- sys/dev/ic/ahci.c +++ sys/dev/ic/ahci.c @@ -3180,6 +3180,8 @@ ahci_hibernate_io(dev_t dev, daddr_t blkno, vaddr_t addr, size_t size, /* map dev to an ahci port */ disk = disk_lookup(&sd_cd, DISKUNIT(dev)); + if (disk == NULL) + return (ENXIO); scsibus = disk->dv_parent; sc = (struct ahci_softc *)disk->dv_parent->dv_parent; @@ -3200,21 +3202,24 @@ ahci_hibernate_io(dev_t dev, daddr_t blkno, vaddr_t addr, size_t size, } if (port == -1) { /* don't know where the disk is */ + device_unref(disk); return (EIO); } my->ap = sc->sc_ports[port]; - + /* we're going to use the first command slot, * so ensure it's not already in use */ if (my->ap->ap_ccbs[0].ccb_xa.state != ATA_S_PUT) { /* this shouldn't happen, we should be idle */ + device_unref(disk); return (EIO); } /* stop the port so we can relocate to the hibernate page */ if (ahci_port_stop(my->ap, 1)) { + device_unref(disk); return (EIO); } ahci_pwrite(my->ap, AHCI_PREG_SCTL, 0); @@ -3237,6 +3242,7 @@ ahci_hibernate_io(dev_t dev, daddr_t blkno, vaddr_t addr, size_t size, /* Check whether port activated. */ cmd = ahci_pread(my->ap, AHCI_PREG_CMD) & ~AHCI_PREG_CMD_ICC; if (!ISSET(cmd, AHCI_PREG_CMD_FRE)) { + device_unref(disk); return (EIO); } @@ -3277,6 +3283,7 @@ ahci_hibernate_io(dev_t dev, daddr_t blkno, vaddr_t addr, size_t size, ahci_pwait_clr(my->ap, AHCI_PREG_CMD, AHCI_PREG_CMD_ICC, 1); if (ahci_port_start(my->ap, 0)) { + device_unref(disk); return (EIO); } @@ -3286,6 +3293,7 @@ ahci_hibernate_io(dev_t dev, daddr_t blkno, vaddr_t addr, size_t size, ahci_write(sc, AHCI_REG_IS, 1 << port); ahci_enable_interrupts(my->ap); + device_unref(disk); return (0); } else if (op == HIB_DONE) { ahci_activate(&my->ap->ap_sc->sc_dev, DVACT_RESUME); diff --git sys/dev/ic/cac.c sys/dev/ic/cac.c index 4c299cd..1312d4f 100644 --- sys/dev/ic/cac.c +++ sys/dev/ic/cac.c @@ -305,6 +305,7 @@ cac_shutdown(void *cookie) if ((sc = (struct cac_softc *)device_lookup(&cac_cd, i)) == NULL) continue; cac_flush(sc); + device_unref(&sc->sc_dv); } } diff --git sys/dev/pci/cz.c sys/dev/pci/cz.c index 9f74393..03b0fb5 100644 --- sys/dev/pci/cz.c +++ sys/dev/pci/cz.c @@ -848,23 +848,30 @@ cztty_getttysoftc(dev_t dev) { int i, j, k, u = minor(dev) & ~CZTTYDIALOUT_MASK; struct cz_softc *cz; + struct cztty_softc *sc; for (i = 0, j = 0; i < cz_cd.cd_ndevs; i++) { k = j; cz = (struct cz_softc *)device_lookup(&cz_cd, i); if (cz == NULL) continue; - if (cz->cz_ports == NULL) + if (cz->cz_ports == NULL) { + device_unref(&cz->cz_dev); continue; + } j += cz->cz_nchannels; if (j > u) break; } - if (i >= cz_cd.cd_ndevs) + if (i >= cz_cd.cd_ndevs) { + device_unref(&cz->cz_dev); return (NULL); - else - return (&cz->cz_ports[u - k]); + } else { + sc = &cz->cz_ports[u - k]; + device_unref(&cz->cz_dev); + return (sc); + } } int