Hi all,

I think I found some cases where the "oldlenp" value is not properly
updated according to the details in the manual page. In particular
this section from sysctl(2):

     The size of the available data can be determined by calling sysctl() with
     a NULL parameter for oldp.  The size of the available data will be
     returned in the location pointed to by oldlenp.

This seems to work fine for most cases, but when testing the program
below I found that for some MIB paths, the oldlenp value is not
updated.

System details: OpenBSD 6.9, amd64, with all patches applied via syspatch

---8<----
#include <stdio.h>

#include <sys/queue.h>
#include <sys/types.h>
#include <sys/sysctl.h>
#include <sys/socket.h>

#include <netinet/in.h>
#include <netinet/tcp.h>
#include <netinet/tcp_timer.h>
#include <netinet/tcp_var.h>

int main(int argc, char **argv)
{
#if 1
        /* working case, prints: "r: 0, len: 4" */
        int path[] = { CTL_NET, PF_INET, IPPROTO_TCP, TCPCTL_SYN_HASH_SIZE };
#else
        /* failing case, prints: "r: 0, len: 0" */
        int path[] = { CTL_NET, PF_INET, IPPROTO_TCP, TCPCTL_BADDYNAMIC };
#endif

        size_t buflen = 0;
        int r = sysctl(path, sizeof(path) / sizeof(path[0]), NULL, &buflen, 
NULL, 0);
        printf("r: %d, len: %zu\n", r, buflen);

        return 0;
}
---8<----

The "TCPCTL_SYN_HASH_SIZE" MIB path ends up being handled by function
sysctl_int_bounded():

---8<----
int
sysctl_int_bounded(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
    int *valp, int minimum, int maximum)
{
        int error = 0;
        int val;

        if (oldp && *oldlenp < sizeof(int))
                return (ENOMEM);
        if (newp && newlen != sizeof(int))
                return (EINVAL);
        *oldlenp = sizeof(int);        <== oldlenp updated
        ...
---8<----

The "TCPCTL_BADDYNAMIC" MIB path ends up being handled by function
sysctl_struct():

---8<----
int
sysctl_struct(void *oldp, size_t *oldlenp, void *newp, size_t newlen, void *sp,
    size_t len)
{
        int error = 0;

        if (oldp && *oldlenp < len)
                return (ENOMEM);
        if (newp && newlen > len)
                return (EINVAL);
        if (oldp) {
                *oldlenp = len;      <== value only conditionally updated
                error = copyout(sp, oldp, len);
        }
        if (error == 0 && newp)
                error = copyin(newp, sp, len);
        return (error);
}
---8<----

Should this update to oldlenp be moved up (something like the patch
below)? Or this intentional / expected behavior?

Thanks,
Nate

---8<----
Index: kern/kern_sysctl.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.389
diff -u -p -r1.389 kern_sysctl.c
--- kern/kern_sysctl.c  8 Feb 2021 10:51:02 -0000       1.389
+++ kern/kern_sysctl.c  4 Oct 2021 01:24:34 -0000
@@ -1043,8 +1043,8 @@ sysctl_struct(void *oldp, size_t *oldlen
                return (ENOMEM);
        if (newp && newlen > len)
                return (EINVAL);
+       *oldlenp = len;
        if (oldp) {
-               *oldlenp = len;
                error = copyout(sp, oldp, len);
        }
        if (error == 0 && newp)
---8<----

Reply via email to