system/freebsd-src
system/freebsd-src
1
0
Fork 0
mirror of https://github.com/freebsd/freebsd-src synced 2024-07-22 02:37:15 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

devfs: add integrity asserts for cdevp_list

Browse source 
It's possible for misuse of cdev KPIs or for bugs in devfs itself to
result in e.g. a cdev object's container being freed while still on the
global list used to populate each devfs mount; see PR 273418 for a
recent example.

Since a node may be marked inactive well before it is reaped from the
list, add a new flag solely to track list membership, and employ it in
some basic list integrity assertions to catch bad actors.

Discussed with:	kib, mjg
MFC after:	1 week
This commit is contained in:
Jason A. Harmening 2023-09-19 08:44:34 -05:00
parent d26c1a0f8b
commit 67864268da
4 changed files with 18 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
sys/fs/devfs/devfs_devs.c
View file

@ -175,6 +175,9 @@ devfs_free(struct cdev *cdev)
struct cdev_priv *cdp;
cdp = cdev2priv(cdev);
KASSERT((cdp->cdp_flags & (CDP_ACTIVE | CDP_ON_ACTIVE_LIST)) == 0,
("%s: cdp %p (%s) still on active list",
__func__, cdp, cdev->si_name));
if (cdev->si_cred != NULL)
crfree(cdev->si_cred);
devfs_free_cdp_inode(cdp->cdp_inode);
@ -516,6 +519,9 @@ devfs_populate_loop(struct devfs_mount *dm, int cleanup)
dev_lock();
TAILQ_FOREACH(cdp, &cdevp_list, cdp_list) {
KASSERT(cdp->cdp_dirents != NULL, ("NULL cdp_dirents"));
KASSERT((cdp->cdp_flags & CDP_ON_ACTIVE_LIST) != 0,
("%s: cdp %p (%s) should not be on active list",
__func__, cdp, cdp->cdp_c.si_name));
/*
* If we are unmounting, or the device has been destroyed,
@ -547,6 +553,7 @@ devfs_populate_loop(struct devfs_mount *dm, int cleanup)
if (!(cdp->cdp_flags & CDP_ACTIVE)) {
if (cdp->cdp_inuse > 0)
continue;
cdp->cdp_flags &= ~CDP_ON_ACTIVE_LIST;
TAILQ_REMOVE(&cdevp_list, cdp, cdp_list);
dev_unlock();
dev_rel(&cdp->cdp_c);
@ -698,7 +705,10 @@ devfs_create(struct cdev *dev)
dev_lock_assert_locked();
cdp = cdev2priv(dev);
cdp->cdp_flags |= CDP_ACTIVE;
KASSERT((cdp->cdp_flags & CDP_ON_ACTIVE_LIST) == 0,
("%s: cdp %p (%s) already on active list",
__func__, cdp, dev->si_name));
cdp->cdp_flags |= (CDP_ACTIVE | CDP_ON_ACTIVE_LIST);
cdp->cdp_inode = alloc_unrl(devfs_inos);
dev_refl(dev);
TAILQ_INSERT_TAIL(&cdevp_list, cdp, cdp_list);

1
sys/fs/devfs/devfs_int.h
View file

@ -55,6 +55,7 @@ struct cdev_priv {
#define CDP_ACTIVE (1 << 0)
#define CDP_SCHED_DTR (1 << 1)
#define CDP_UNREF_DTR (1 << 2)
#define CDP_ON_ACTIVE_LIST (1 << 3)
u_int cdp_inuse;
u_int cdp_maxdirent;

4
sys/fs/devfs/devfs_vnops.c
View file

@ -1664,6 +1664,10 @@ devfs_revoke(struct vop_revoke_args *ap)
dev_lock();
cdp->cdp_inuse--;
if (!(cdp->cdp_flags & CDP_ACTIVE) && cdp->cdp_inuse == 0) {
KASSERT((cdp->cdp_flags & CDP_ON_ACTIVE_LIST) != 0,
("%s: cdp %p (%s) not on active list",
__func__, cdp, dev->si_name));
cdp->cdp_flags &= ~CDP_ON_ACTIVE_LIST;
TAILQ_REMOVE(&cdevp_list, cdp, cdp_list);
dev_unlock();
dev_rel(&cdp->cdp_c);

2
sys/kern/kern_conf.c
View file

@ -119,6 +119,8 @@ dev_free_devlocked(struct cdev *cdev)
cdp = cdev2priv(cdev);
KASSERT((cdp->cdp_flags & CDP_UNREF_DTR) == 0,
("destroy_dev() was not called after delist_dev(%p)", cdev));
KASSERT((cdp->cdp_flags & CDP_ON_ACTIVE_LIST) == 0,
("%s: cdp %p (%s) on active list", __func__, cdp, cdev->si_name));
TAILQ_INSERT_HEAD(&cdevp_free_list, cdp, cdp_list);
}