Sun Apr 16 11:14:58 2023 UTC ()

autoconf(9): Avoid potential ABA bug in config_makeroom.

When we unlock alldevs_lock to allocate a new cd_devs array nsp,
other threads may have:

1. freed the old one (osp),
2. done some other memory allocation,
3. allocated a new _larger_ array whose address happens to concide
   with osp (e.g., in (2) the page was recycled for a different pool
   cache), and
4. updated cd_devs back to osp but increased cd_ndevs.

In that case, the memory may be corrupted: we try to copy the wrong
number of device_t pointers into nsp and we free osp with the wrong
(stale) length.

Avoid this by checking whether cd_ndevs has changed too -- if not,
osp might have been recycled but at least the lengths we're about to
copy and free are still correct so there's no harm in an ABA
situation.

XXX pullup-8
XXX pullup-9
XXX pullup-10


(riastradh)

diff -r1.307 -r1.308 src/sys/kern/subr_autoconf.c

--- src/sys/kern/subr_autoconf.c 2023/02/22 17:00:16	1.307
+++ src/sys/kern/subr_autoconf.c 2023/04/16 11:14:58	1.308

 @@ -1,14 +1,14 @@
-/* $NetBSD: subr_autoconf.c,v 1.307 2023/02/22 17:00:16 riastradh Exp $ */
+/* $NetBSD: subr_autoconf.c,v 1.308 2023/04/16 11:14:58 riastradh Exp $ */
 /*
  * Copyright (c) 1996, 2000 Christopher G. Demetriou
  * All rights reserved.
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
 @@ -67,27 +67,27 @@
  * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
  * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
  * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
+ *
  * from: Header: subr_autoconf.c,v 1.12 93/02/01 19:31:48 torek Exp  (LBL)
+ *
  *	@(#)subr_autoconf.c	8.3 (Berkeley) 5/17/94
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.307 2023/02/22 17:00:16 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.308 2023/04/16 11:14:58 riastradh Exp $");
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
 #include "drvctl.h"
 #endif
 #include <sys/param.h>
 #include <sys/device.h>
 #include <sys/device_impl.h>
 #include <sys/disklabel.h>
 #include <sys/conf.h>
 #include <sys/kauth.h>
 #include <sys/kmem.h>
 @@ -1347,27 +1347,27 @@ config_makeroom(int n, struct cfdriver *
 		/*
 		 * Release alldevs_lock around allocation, which may
 		 * sleep.
 		 */
 		mutex_exit(&alldevs_lock);
 		nsp = kmem_alloc(sizeof(device_t) * nndevs, KM_SLEEP);
 		mutex_enter(&alldevs_lock);
 		/*
 		 * If another thread moved the array while we did
 		 * not hold alldevs_lock, try again.
 		 */
-		if (cd->cd_devs != osp) {
+		if (cd->cd_devs != osp || cd->cd_ndevs != ondevs) {
 			mutex_exit(&alldevs_lock);
 			kmem_free(nsp, sizeof(device_t) * nndevs);
 			mutex_enter(&alldevs_lock);
 			continue;
+		}
 		memset(nsp + ondevs, 0, sizeof(device_t) * (nndevs - ondevs));
 		if (ondevs != 0)
 			memcpy(nsp, cd->cd_devs, sizeof(device_t) * ondevs);
 		cd->cd_ndevs = nndevs;
 		cd->cd_devs = nsp;
 		if (ondevs != 0) {