git: 23fe1083ca8c - stable/13 - libc: return partial sysctl() result if buffer is too small

From: Stefan Eßer <se_at_FreeBSD.org>
Date: Fri, 04 Mar 2022 19:55:04 UTC
The branch stable/13 has been updated by se:

URL: https://cgit.FreeBSD.org/src/commit/?id=23fe1083ca8cd10d28d057cfa558c28bb3bf19d3

commit 23fe1083ca8cd10d28d057cfa558c28bb3bf19d3
Author:     Stefan Eßer <se@FreeBSD.org>
AuthorDate: 2022-02-04 12:44:20 +0000
Commit:     Stefan Eßer <se@FreeBSD.org>
CommitDate: 2022-03-04 19:54:00 +0000

    libc: return partial sysctl() result if buffer is too small
    
    Testing of a new feature revealed that calling sysctl() to retrieve
    the value of the user.localbase variable passing too low a buffer size
    could leave the result buffer unchanged.
    
    The behavior in the normal case of a sufficiently large buffer was
    correct.
    
    All known callers pass a sufficiently large buffer and have thus not
    been affected by this issue. If a non-default value had been assigned
    to this variable, the result was as documented, too.
    
    Fix the function to fill the buffer with a partial result, if the
    passed in buffer size is too low to hold the full result.
    
    (cherry picked from commit e11ad014d1468729ecf758ab3709618a78feae1b)
    
    libc: add helper furnction to set sysctl() user.* variables
    
    Testing had revealed that trying to retrieve the user.localbase
    variable into to small a buffer would return the correct error code,
    but would not fill the available buffer space with a partial result.
    
    A partial result is of no use, but this is still a violation of the
    documented behavior, which has been fixed in the previous commit to
    this function.
    
    I just checked the code for "user.cs_path" and found that it had the
    same issue.
    
    Instead of fixing the logic for each user.* sysctl string variable
    individually, this commit adds a helper function set_user_str() that
    implements the semantics specified in the sysctl() man page.
    
    It is currently only used for "user.cs_path" and "user.localbase",
    but it will offer a significant simplification when further such
    variables will be added (as I intend to do).
    
    (cherry picked from commit 9535d9f104d82487abfc3a64de18f9d010bba6d2)
    
    sysctlbyname(): restore access to user variables
    
    The optimization of sysctlbyname() in commit d05b53e0baee7 had the
    side-effect of not going through the fix-up for the user.* variables
    in the previously called sysctl() function.
    
    This lead to 0 or an empty strings being returned by sysctlbyname()
    for all user.* variables.
    
    An alternate implementation would store the user variables in the
    kernel during system start-up. That would allow to remove the fix-up
    code in the C library that is currently required to provide the actual
    values.
    
    This update restores the previous code path for the user.* variables
    and keeps the performance optimization intact for all other variables.
    
    (cherry picked from commit af7d105379a649b7af4bffd15fbeab692bb52b69)
---
 lib/libc/gen/sysctl.c       | 42 ++++++++++++++++++++++++------------------
 lib/libc/gen/sysctlbyname.c | 15 ++++++++++++---
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/lib/libc/gen/sysctl.c b/lib/libc/gen/sysctl.c
index cdd2d3e391e6..59242b50bbed 100644
--- a/lib/libc/gen/sysctl.c
+++ b/lib/libc/gen/sysctl.c
@@ -46,6 +46,25 @@ __FBSDID("$FreeBSD$");
 extern int __sysctl(const int *name, u_int namelen, void *oldp,
     size_t *oldlenp, const void *newp, size_t newlen);
 
+static int
+set_user_str(void *dstp, size_t *dstlenp, const char *src, size_t len,
+    size_t maxlen)
+{
+	int retval;
+
+	retval = 0;
+	if (dstp != NULL) {
+		if (len > maxlen) {
+			len = maxlen;
+			errno = ENOMEM;
+			retval = -1;
+		}
+		memcpy(dstp, src, len);
+	}
+	*dstlenp = len;
+	return (retval);
+}
+
 int
 sysctl(const int *name, u_int namelen, void *oldp, size_t *oldlenp,
     const void *newp, size_t newlen)
@@ -74,17 +93,10 @@ sysctl(const int *name, u_int namelen, void *oldp, size_t *oldlenp,
 	/* Variables under CLT_USER that may be overridden by kernel values */
 	switch (name[1]) {
 	case USER_LOCALBASE:
-		if (oldlenp == NULL || *oldlenp != 1)
+		if (oldlenp == NULL || *oldlenp > sizeof(""))
 			return (0);
-		if (oldp != NULL) {
-			if (orig_oldlen < sizeof(_PATH_LOCALBASE)) {
-				errno = ENOMEM;
-				return (-1);
-			}
-			memmove(oldp, _PATH_LOCALBASE, sizeof(_PATH_LOCALBASE));
-		}
-		*oldlenp = sizeof(_PATH_LOCALBASE);
-		return (0);
+		return (set_user_str(oldp, oldlenp, _PATH_LOCALBASE,
+			sizeof(_PATH_LOCALBASE), orig_oldlen));
 	}
 
 	/* Variables under CLT_USER whose values are immutably defined below */
@@ -95,14 +107,8 @@ sysctl(const int *name, u_int namelen, void *oldp, size_t *oldlenp,
 
 	switch (name[1]) {
 	case USER_CS_PATH:
-		if (oldp != NULL && orig_oldlen < sizeof(_PATH_STDPATH)) {
-			errno = ENOMEM;
-			return (-1);
-		}
-		*oldlenp = sizeof(_PATH_STDPATH);
-		if (oldp != NULL)
-			memmove(oldp, _PATH_STDPATH, sizeof(_PATH_STDPATH));
-		return (0);
+		return (set_user_str(oldp, oldlenp, _PATH_STDPATH,
+		    sizeof(_PATH_STDPATH), orig_oldlen));
 	}
 
 	if (oldp != NULL && *oldlenp < sizeof(int)) {
diff --git a/lib/libc/gen/sysctlbyname.c b/lib/libc/gen/sysctlbyname.c
index 9b4ffc0ca4ae..5086cc4b7d81 100644
--- a/lib/libc/gen/sysctlbyname.c
+++ b/lib/libc/gen/sysctlbyname.c
@@ -41,8 +41,17 @@ sysctlbyname(const char *name, void *oldp, size_t *oldlenp,
     const void *newp, size_t newlen)
 {
 	size_t len;
+	int oid[2];
 
-	len = strlen(name);
-	return (__sysctlbyname(name, len, oldp, oldlenp, newp,
-	    newlen));
+	if (__predict_true(strncmp(name, "user.", 5) != 0)) {
+		len = strlen(name);
+		return (__sysctlbyname(name, len, oldp, oldlenp, newp,
+			newlen));
+	} else {
+		len = nitems(oid);
+		if (sysctlnametomib(name, oid, &len) == -1)
+			return (-1);
+		return (sysctl(oid, (u_int)len, oldp, oldlenp, newp,
+		    newlen));
+	}
 }