NetBSD Problem Report #57996

From www@netbsd.org  Sun Mar  3 22:51:41 2024
Return-Path: <www@netbsd.org>
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 3993A1A9239
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  3 Mar 2024 22:51:41 +0000 (UTC)
Message-Id: <20240303225140.382EF1A923A@mollari.NetBSD.org>
Date: Sun,  3 Mar 2024 22:51:40 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: sysctl kern.ccd.info and kern.ccd.components abuse sysctl interface
X-Send-Pr-Version: www-1.0

>Number:         57996
>Category:       kern
>Synopsis:       sysctl kern.ccd.info and kern.ccd.components abuse sysctl interface
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Mar 03 22:55:00 +0000 2024
>Last-Modified:  Tue Mar 05 20:50:01 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, 8, 7, ...
>Organization:
The NetCCD Foundation
>Environment:
>Description:
The sysctl nodes kern.ccd.info and kern.ccd.components, which are used by `ccdconfig -g', work as follows:

- As `oldp' (old-data pointer), userland passes a pointer to buffer that is to be filled with the information about a ccd instance.
- As `newp' (new-data pointer), userland passes a pointer to an int that specifies _which_ ccd instance.
- The kernel reads out *(const int *)newp to decide which instance to read out, then writes to oldp on the basis of that.

This is an abuse of the sysctl interface.  The unit information is not actually being written, and the sysctls don't (or shouldn't) have side effects, so there's no need for the knobs to be writable.

Instead, this information should be exposed through one child node per ccd(4) instance, like sysctl hw.ccdN.info and hw.ccdN.components.
>How-To-Repeat:
code inspection
>Fix:
Yes, please!  Separate sysctl knobs per ccd(4) instance.

>Audit-Trail:
From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/57996 sysctl kern.ccd.info and kern.ccd.components abuse
 sysctl interface
Date: Tue, 5 Mar 2024 12:50:23 -0800 (PST)

 The following patch works around the problem, confirming that the
 diagnosis is correct.

 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 |
 +---------------------+--------------------------+----------------------+

Home	PR Database Search