d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix two races in the handling of the d_gianttrick for the D_NEEDGIANT

Browse Source 
drivers.

In the giant_XXX wrappers for the device methods of the D_NEEDGIANT
drivers, do not dereference the cdev->si_devsw. It is racing with
the destroy_devl() clearing of the si_devsw. Instead, use the
dev_refthread() and return ENXIO for the destroyed device. [1]

The check for the D_INIT in the prep_cdevsw() was not synchronized with
the call of the fini_cdevsw() in destroy_devl(), that under rapid device
creation/destruction may result in the use of uninitialized cdevsw [2].
Change the protocol for the prep_cdevsw(), requiring it to be called
under dev_mtx, where the check for D_INIT is done.

Do not free the memory allocated for the gianttrick cdevsw while holding
the dev_mtx, put it into the free list to be freed later. Reuse the
d_gianttrick pointer to keep the size and layout of the struct cdevsw
(requested by phk). Free the memory in the dev_unlock_and_free(), and do
all the free after the dev_mtx is dropped (suggested by jhb).

Reported by:	bsdimp + many [1], pho [2]
Reviewed by:	phk, jhb
Tested by:	pho
MFC after:	1 week
This commit is contained in:
Konstantin Belousov 2008-03-17 13:17:10 +00:00
parent c2877015a1
commit aeeb4202df
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=177301
2 changed files with 128 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

158
sys/kern/kern_conf.c
View File

@ -61,6 +61,8 @@ static struct cdev *make_dev_credv(int flags,
static struct cdev_priv_list cdevp_free_list =
TAILQ_HEAD_INITIALIZER(cdevp_free_list);
static SLIST_HEAD(free_cdevsw, cdevsw) cdevsw_gt_post_list =
SLIST_HEAD_INITIALIZER();
void
dev_lock(void)
@ -69,19 +71,41 @@ dev_lock(void)
mtx_lock(&devmtx);
}
/*
* Free all the memory collected while the cdev mutex was
* locked. Since devmtx is after the system map mutex, free() cannot
* be called immediately and is postponed until cdev mutex can be
* dropped.
*/
static void
dev_unlock_and_free(void)
{
struct cdev_priv_list cdp_free;
struct free_cdevsw csw_free;
struct cdev_priv *cdp;
struct cdevsw *csw;
mtx_assert(&devmtx, MA_OWNED);
while ((cdp = TAILQ_FIRST(&cdevp_free_list)) != NULL) {
TAILQ_REMOVE(&cdevp_free_list, cdp, cdp_list);
mtx_unlock(&devmtx);
devfs_free(&cdp->cdp_c);
mtx_lock(&devmtx);
}
/*
* Make the local copy of the list heads while the dev_mtx is
* held. Free it later.
*/
TAILQ_INIT(&cdp_free);
TAILQ_CONCAT(&cdp_free, &cdevp_free_list, cdp_list);
csw_free = cdevsw_gt_post_list;
SLIST_INIT(&cdevsw_gt_post_list);
mtx_unlock(&devmtx);
while ((cdp = TAILQ_FIRST(&cdp_free)) != NULL) {
TAILQ_REMOVE(&cdp_free, cdp, cdp_list);
devfs_free(&cdp->cdp_c);
}
while ((csw = SLIST_FIRST(&csw_free)) != NULL) {
SLIST_REMOVE_HEAD(&csw_free, d_postfree_list);
free(csw, M_DEVT);
}
}
static void
@ -94,6 +118,14 @@ dev_free_devlocked(struct cdev *cdev)
TAILQ_INSERT_HEAD(&cdevp_free_list, cdp, cdp_list);
}
static void
cdevsw_free_devlocked(struct cdevsw *csw)
{
mtx_assert(&devmtx, MA_OWNED);
SLIST_INSERT_HEAD(&cdevsw_gt_post_list, csw, d_postfree_list);
}
void
dev_unlock(void)
{
@ -297,118 +329,164 @@ no_poll(struct cdev *dev __unused, int events, struct thread *td __unused)
static int
giant_open(struct cdev *dev, int oflags, int devtype, struct thread *td)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_open(dev, oflags, devtype, td);
retval = dsw->d_gianttrick->d_open(dev, oflags, devtype, td);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
static int
giant_fdopen(struct cdev *dev, int oflags, struct thread *td, struct file *fp)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_fdopen(dev, oflags, td, fp);
retval = dsw->d_gianttrick->d_fdopen(dev, oflags, td, fp);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
static int
giant_close(struct cdev *dev, int fflag, int devtype, struct thread *td)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_close(dev, fflag, devtype, td);
retval = dsw->d_gianttrick->d_close(dev, fflag, devtype, td);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
static void
giant_strategy(struct bio *bp)
{
struct cdevsw *dsw;
struct cdev *dev;
dev = bp->bio_dev;
dsw = dev_refthread(dev);
if (dsw == NULL) {
biofinish(bp, NULL, ENXIO);
return;
}
mtx_lock(&Giant);
bp->bio_dev->si_devsw->d_gianttrick->
d_strategy(bp);
dsw->d_gianttrick->d_strategy(bp);
mtx_unlock(&Giant);
dev_relthread(dev);
}
static int
giant_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int fflag, struct thread *td)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_ioctl(dev, cmd, data, fflag, td);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
static int
giant_read(struct cdev *dev, struct uio *uio, int ioflag)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_read(dev, uio, ioflag);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
static int
giant_write(struct cdev *dev, struct uio *uio, int ioflag)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_write(dev, uio, ioflag);
retval = dsw->d_gianttrick->d_write(dev, uio, ioflag);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
static int
giant_poll(struct cdev *dev, int events, struct thread *td)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_poll(dev, events, td);
retval = dsw->d_gianttrick->d_poll(dev, events, td);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
static int
giant_kqfilter(struct cdev *dev, struct knote *kn)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_kqfilter(dev, kn);
retval = dsw->d_gianttrick->d_kqfilter(dev, kn);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
static int
giant_mmap(struct cdev *dev, vm_offset_t offset, vm_paddr_t *paddr, int nprot)
{
struct cdevsw *dsw;
int retval;
dsw = dev_refthread(dev);
if (dsw == NULL)
return (ENXIO);
mtx_lock(&Giant);
retval = dev->si_devsw->d_gianttrick->
d_mmap(dev, offset, paddr, nprot);
retval = dsw->d_gianttrick->d_mmap(dev, offset, paddr, nprot);
mtx_unlock(&Giant);
dev_relthread(dev);
return (retval);
}
@ -490,7 +568,7 @@ fini_cdevsw(struct cdevsw *devsw)
if (devsw->d_gianttrick != NULL) {
gt = devsw->d_gianttrick;
memcpy(devsw, gt, sizeof *devsw);
free(gt, M_DEVT);
cdevsw_free_devlocked(gt);
devsw->d_gianttrick = NULL;
}
devsw->d_flags &= ~D_INIT;
@ -501,11 +579,20 @@ prep_cdevsw(struct cdevsw *devsw)
{
struct cdevsw *dsw2;
if (devsw->d_flags & D_NEEDGIANT)
mtx_assert(&devmtx, MA_OWNED);
if (devsw->d_flags & D_INIT)
return;
if (devsw->d_flags & D_NEEDGIANT) {
dev_unlock();
dsw2 = malloc(sizeof *dsw2, M_DEVT, M_WAITOK);
else
dev_lock();
} else
dsw2 = NULL;
dev_lock();
if (devsw->d_flags & D_INIT) {
if (dsw2 != NULL)
cdevsw_free_devlocked(dsw2);
return;
}
if (devsw->d_version != D_VERSION_01) {
printf(
@ -536,8 +623,8 @@ prep_cdevsw(struct cdevsw *devsw)
if (devsw->d_gianttrick == NULL) {
memcpy(dsw2, devsw, sizeof *dsw2);
devsw->d_gianttrick = dsw2;
} else
free(dsw2, M_DEVT);
dsw2 = NULL;
}
}
#define FIXUP(member, noop, giant) \
@ -566,7 +653,8 @@ prep_cdevsw(struct cdevsw *devsw)
devsw->d_flags |= D_INIT;
dev_unlock();
if (dsw2 != NULL)
cdevsw_free_devlocked(dsw2);
}
struct cdev *
@ -580,10 +668,9 @@ make_dev_credv(int flags, struct cdevsw *devsw, int minornr,
KASSERT((minornr & ~MAXMINOR) == 0,
("Invalid minor (0x%x) in make_dev", minornr));
if (!(devsw->d_flags & D_INIT))
prep_cdevsw(devsw);
dev = devfs_alloc();
dev_lock();
prep_cdevsw(devsw);
dev = newdev(devsw, minornr, dev);
if (flags & MAKEDEV_REF)
dev_refl(dev);
@ -620,7 +707,7 @@ make_dev_credv(int flags, struct cdevsw *devsw, int minornr,
devfs_create(dev);
clean_unrhdrl(devfs_inos);
dev_unlock();
dev_unlock_and_free();
return (dev);
}
@ -884,8 +971,6 @@ clone_create(struct clonedevs **cdp, struct cdevsw *csw, int *up, struct cdev **
KASSERT(*up <= CLONE_UNITMASK,
("Too high unit (0x%x) in clone_create", *up));
if (!(csw->d_flags & D_INIT))
prep_cdevsw(csw);
/*
* Search the list for a lot of things in one go:
@ -898,6 +983,7 @@ clone_create(struct clonedevs **cdp, struct cdevsw *csw, int *up, struct cdev **
unit = *up;
ndev = devfs_alloc();
dev_lock();
prep_cdevsw(csw);
low = extra;
de = dl = NULL;
cd = *cdp;
@ -977,7 +1063,7 @@ clone_cleanup(struct clonedevs **cdp)
destroy_devl(dev);
}
}
dev_unlock();
dev_unlock_and_free();
free(cd, M_DEVBUF);
*cdp = NULL;
}
@ -1004,7 +1090,7 @@ destroy_dev_tq(void *ctx, int pending)
cb = cp->cdp_dtr_cb;
cb_arg = cp->cdp_dtr_cb_arg;
destroy_devl(dev);
dev_unlock();
dev_unlock_and_free();
dev_rel(dev);
if (cb != NULL)
cb(cb_arg);

7
sys/sys/conf.h
View File

@ -213,8 +213,13 @@ struct cdevsw {
LIST_ENTRY(cdevsw) d_list;
LIST_HEAD(, cdev) d_devs;
int d_spare3;
struct cdevsw *d_gianttrick;
union {
struct cdevsw *gianttrick;
SLIST_ENTRY(cdevsw) postfree_list;
} __d_giant;
};
#define d_gianttrick __d_giant.gianttrick
#define d_postfree_list __d_giant.postfree_list
#define NUMCDEVSW 256