From nobody Thu Sep 28 01:28:43 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4RwwqJ1W7Xz4vHSn; Thu, 28 Sep 2023 01:28:44 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RwwqH74SJz3Hhq; Thu, 28 Sep 2023 01:28:43 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1695864524; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=nzBXCUfWeiYCOc2Ox920kWeZcOgemYEDMOE2ENPMIfw=; b=rYcGzUI11WNU8J7Hs9IHogrbmjFkZZY+EXKdl91kduZQ2OP8Gd4PIMgBrDBT/yVUCPKp7i Z6P/4uBlb5TjMI92HYcEQk3LNuWH8pfUCa4hqK4hCQIdIud0aFeZXSTpzwivBMefdfcZdt XtE6zfd4+sXro8CWFOXj9iCOcK3g+V6IlehvON8Dn//xuGKx+5c31v42m4THOLtL9Oz7kC 04DnMBe3glfoo+XYe/Ui2pAqbWtG/WDtIvwXLRKTW8LZ7b6rBME+mfe5cNEOmnhSOPTrHt 1H4B+Alb3qZpocKSVv61vtWQ7SpRga4jSYPt8+Fse/EHVSQbWOw7rP0qPGlqlw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1695864524; a=rsa-sha256; cv=none; b=XGPj0kE3GV/FhNmDPb1iZwXE7oVJ5VGhUDLV/V11e4amBbM/ht1CDHGTHgbR5qVkDSYBH2 AuBnepiTjlDG/t/DarvVV+AR5uTQPNsEQ+dVEAd8thSXKEEeyfbv9merpQnfaMko0dHrYo DYpOS2bK9wLO6jXDR7dWUzJZWtdUFm+ibSz37I7PkZqwskHSLT4zTqPG4zGsU6Om67QxQ4 zlJT9b3JbLKDplHBgdzPlWX3g89UEtiTHBrZUhAJF1LBf0F0mcw6RhrtZCyrYrUJCGSPYP wbj5QUf2bs0gRK+y++F5ZtfBzAKJVxS80iI2JvWSmVgaSZ0jJnpp1WZWGfKp6A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1695864524; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=nzBXCUfWeiYCOc2Ox920kWeZcOgemYEDMOE2ENPMIfw=; b=XXfAbl8ebjk7sBV9zLVanNXv5tAugmez+NSjTQQHaawW/6q/GJ+OovK40zoq0c4wcD0sBs hnaPGT8qrtpldqR1aJhHU7+UJPPZp+ervYEA0/2baiXyiDlMG43lfvjt5HyDHKepAoXnp0 /jbxmEMprgwUYCcJiy/QWlvAyEuduxW6ckq+V70WxG1/lKlunreAq+5Ah1uutsNeADMoEu 3s1yeNnGb8IIHOvK2lrorFiruMICghwwO1LGxTWsGNXFwsedfHU/Aw+rhieltaqdMEHHzj hpCx7IUe1kgXc17Aan4u4AsyvZyYO4MYk2VSlxhwivJFXr2trXg5h97ffet0yw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4RwwqH69Z8z11y9; Thu, 28 Sep 2023 01:28:43 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 38S1Sh6o073481; Thu, 28 Sep 2023 01:28:43 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 38S1Shdg073478; Thu, 28 Sep 2023 01:28:43 GMT (envelope-from git) Date: Thu, 28 Sep 2023 01:28:43 GMT Message-Id: <202309280128.38S1Shdg073478@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: "Jason A. Harmening" Subject: git: 23332e34e653 - stable/14 - devfs: add integrity asserts for cdevp_list List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jah X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 23332e34e653131c0ff3a948b57d06948102a06b Auto-Submitted: auto-generated The branch stable/14 has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=23332e34e653131c0ff3a948b57d06948102a06b commit 23332e34e653131c0ff3a948b57d06948102a06b Author: Jason A. Harmening AuthorDate: 2023-09-19 13:44:34 +0000 Commit: Jason A. Harmening CommitDate: 2023-09-28 00:46:38 +0000 devfs: add integrity asserts for cdevp_list 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 (cherry picked from commit 67864268da53b792836f13be10299de8cd62997e) --- sys/fs/devfs/devfs_devs.c | 12 +++++++++++- sys/fs/devfs/devfs_int.h | 1 + sys/fs/devfs/devfs_vnops.c | 4 ++++ sys/kern/kern_conf.c | 2 ++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/sys/fs/devfs/devfs_devs.c b/sys/fs/devfs/devfs_devs.c index c6efd0d421b1..db879efe803a 100644 --- a/sys/fs/devfs/devfs_devs.c +++ b/sys/fs/devfs/devfs_devs.c @@ -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); diff --git a/sys/fs/devfs/devfs_int.h b/sys/fs/devfs/devfs_int.h index 32c6fb414250..916297425b53 100644 --- a/sys/fs/devfs/devfs_int.h +++ b/sys/fs/devfs/devfs_int.h @@ -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; diff --git a/sys/fs/devfs/devfs_vnops.c b/sys/fs/devfs/devfs_vnops.c index 2f700f9dad25..1df7d13be919 100644 --- a/sys/fs/devfs/devfs_vnops.c +++ b/sys/fs/devfs/devfs_vnops.c @@ -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); diff --git a/sys/kern/kern_conf.c b/sys/kern/kern_conf.c index d6063696c85b..a7c22b7d118a 100644 --- a/sys/kern/kern_conf.c +++ b/sys/kern/kern_conf.c @@ -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); }