NetBSD Problem Report #58051

From paul@whooppee.com  Tue Mar 19 01:39:54 2024
Return-Path: <paul@whooppee.com>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 4DFE41A9239
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 19 Mar 2024 01:39:54 +0000 (UTC)
Message-Id: <20240319013942.24C3B5E33C5@speedy.whooppee.com>
Date: Mon, 18 Mar 2024 18:39:42 -0700 (PDT)
From: paul@whooppee.com
Reply-To: paul@whooppee.com
To: gnats-bugs@NetBSD.org
Subject: ccd(4) sysctl()s abused the api
X-Send-Pr-Version: 3.95

>Number:         58051
>Category:       kern
>Synopsis:       ccd(4) sysctl()s abused the api
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 19 01:40:00 +0000 2024
>Last-Modified:  Sat Apr 26 21:30:01 +0000 2025
>Originator:     Paul Goyette
>Release:        NetBSD 10.99.10
>Organization:
+---------------------+--------------------------+----------------------+
| Paul Goyette (.sig) | PGP Key fingerprint:     | E-mail addresses:    |
| (Retired)           | 1B11 1849 721C 56C8 F63A | paul@whooppee.com    |
| Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | pgoyette@netbsd.org  |
| & Network Engineer  |                          | pgoyette99@gmail.com |
+---------------------+--------------------------+----------------------+
>Environment:


System: NetBSD speedy.whooppee.com 10.99.10 NetBSD 10.99.10 (SPEEDY 2024-03-18 02:46:01 UTC) #0: Mon Mar 18 05:00:00 UTC 2024 paul@speedy.whooppee.com:/build/netbsd-local/obj/amd64/sys/arch/amd64/compile/SPEEDY amd64
Architecture: x86_64
Machine: amd64
>Description:
	The ccd(4) sysctl()s mistreat the newp argument as an index
	to select a device unit.  On amd64 this results in an SMAP
	access error and panic's the machine.

>How-To-Repeat:
	Configure a ccd(4) then try ``ccdconfig -g''.
>Fix:
	The following works around the problem.  The correct solution
	would need a rewrite of ccd(4)'s sysctl() code.

Index: ccd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ccd.c,v
retrieving revision 1.189
diff -u -p -r1.189 ccd.c
--- ccd.c     28 Mar 2022 12:48:35 -0000      1.189
+++ ccd.c     5 Mar 2024 20:17:50 -0000
@@ -1772,13 +1772,14 @@ ccd_info_sysctl(SYSCTLFN_ARGS)
 	struct sysctlnode node;
 	struct ccddiskinfo ccd;
 	struct ccd_softc *sc;
-	int unit;
+	int unit, error;

 	if (newp == NULL || newlen != sizeof(int))
 		return EINVAL;

-	unit = *(const int *)newp;
-	newp = NULL;
+	error = sysctl_copyin(l, newp, &unit, sizeof unit);
+	if (error)
+		return error;
 	newlen = 0;
 	ccd.ccd_ndisks = ~0;
 	mutex_enter(&ccd_lock);
@@ -1816,8 +1817,9 @@ ccd_components_sysctl(SYSCTLFN_ARGS)
 		return EINVAL;

 	size = 0;
-	unit = *(const int *)newp;
-	newp = NULL;
+	error = sysctl_copyin(l, newp, &unit, sizeof unit);
+	if (error)
+		return error;
 	newlen = 0;
 	mutex_enter(&ccd_lock);
 	LIST_FOREACH(sc, &ccds, sc_link)


>Audit-Trail:
From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/58051: ccd(4) sysctl()s abused the api
Date: Sat, 26 Apr 2025 07:24:48 -0700 (PDT)

 Any reason to keep this PR open?  At the least, can we commit
 the proposed work-around, and then lower the priority?

 >> Fix:
 > 	The following works around the problem.  The correct solution
 > 	would need a rewrite of ccd(4)'s sysctl() code.
 >
 > Index: ccd.c
 > ===================================================================
 > RCS file: /cvsroot/src/sys/dev/ccd.c,v
 > retrieving revision 1.189
 > diff -u -p -r1.189 ccd.c
 > --- ccd.c     28 Mar 2022 12:48:35 -0000      1.189
 > +++ ccd.c     5 Mar 2024 20:17:50 -0000
 > @@ -1772,13 +1772,14 @@ ccd_info_sysctl(SYSCTLFN_ARGS)
 > 	struct sysctlnode node;
 > 	struct ccddiskinfo ccd;
 > 	struct ccd_softc *sc;
 > -	int unit;
 > +	int unit, error;
 >
 > 	if (newp == NULL || newlen != sizeof(int))
 > 		return EINVAL;
 >
 > -	unit = *(const int *)newp;
 > -	newp = NULL;
 > +	error = sysctl_copyin(l, newp, &unit, sizeof unit);
 > +	if (error)
 > +		return error;
 > 	newlen = 0;
 > 	ccd.ccd_ndisks = ~0;
 > 	mutex_enter(&ccd_lock);
 > @@ -1816,8 +1817,9 @@ ccd_components_sysctl(SYSCTLFN_ARGS)
 > 		return EINVAL;
 >
 > 	size = 0;
 > -	unit = *(const int *)newp;
 > -	newp = NULL;
 > +	error = sysctl_copyin(l, newp, &unit, sizeof unit);
 > +	if (error)
 > +		return error;
 > 	newlen = 0;
 > 	mutex_enter(&ccd_lock);
 > 	LIST_FOREACH(sc, &ccds, sc_link)

 +---------------------+--------------------------+----------------------+
 | Paul Goyette (.sig) | PGP Key fingerprint:     | E-mail addresses:    |
 | (Retired)           | 1B11 1849 721C 56C8 F63A | paul@whooppee.com    |
 | Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | pgoyette@netbsd.org  |
 | & Network Engineer  |                          | pgoyette99@gmail.com |
 +---------------------+--------------------------+----------------------+

From: matthew green <mrg@eterna23.net>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org, paul@whooppee.com
Subject: re: kern/58051: ccd(4) sysctl()s abused the api
Date: Sun, 27 Apr 2025 06:27:30 +1000

 >  Any reason to keep this PR open?  At the least, can we commit
 >  the proposed work-around, and then lower the priority?

 haven't you already commited the workaround?

 ---
 revision 1.191
 date: 2024-04-11 22:04:02 -0700;  author: pgoyette;  state: Exp;  lines: +=
 9 -7;  commitid: MDrZuMwMNs6wqO5F;
 Repair handling of ccd sysctls to not use newp as an index.

 This is a work-around for kern/58051.  It is not a complete fix.
 ---


 .mrg.

From: Paul Goyette <paul@whooppee.com>
To: matthew green <mrg@eterna23.net>
Cc: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org, gnats-admin@netbsd.org, 
    netbsd-bugs@netbsd.org
Subject: re: kern/58051: ccd(4) sysctl()s abused the api
Date: Sat, 26 Apr 2025 14:29:06 -0700 (PDT)

 On Sun, 27 Apr 2025, matthew green wrote:

 >>  Any reason to keep this PR open?  At the least, can we commit
 >>  the proposed work-around, and then lower the priority?
 >
 > haven't you already commited the workaround?

 Oh, right!  thanks.

 >
 > ---
 > revision 1.191
 > date: 2024-04-11 22:04:02 -0700;  author: pgoyette;  state: Exp;  lines: +9 -7;  commitid: MDrZuMwMNs6wqO5F;
 > Repair handling of ccd sysctls to not use newp as an index.
 >
 > This is a work-around for kern/58051.  It is not a complete fix.
 > ---
 >
 >
 > .mrg.
 >
 > !DSPAM:680d41d4105923159210108!
 >
 >

 +---------------------+--------------------------+----------------------+
 | Paul Goyette (.sig) | PGP Key fingerprint:     | E-mail addresses:    |
 | (Retired)           | 1B11 1849 721C 56C8 F63A | paul@whooppee.com    |
 | Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | pgoyette@netbsd.org  |
 | & Network Engineer  |                          | pgoyette99@gmail.com |
 +---------------------+--------------------------+----------------------+

>Unformatted:

Home	PR Database Search