ENOMEM when calling sysctl_handle_int from custom handler

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

ENOMEM when calling sysctl_handle_int from custom handler

Austin Shafer
Hi freebsd-hackers,

I've had an issue with creating a custom sysctl handler that I can't
seem to find an answer for. Using sysctl_handle_int as a handler in
SYSCTL_ADD_PROC works fine, but calling sysctl_handle_int from a custom
sysctl handler results in an error.

(sorry for the long message, I tried to be verbose)

Creating the nodes works fine, but SYSCTL_OUT or sysctl_handle_int
return ENOMEM despite there being plenty of available memory. I based my
approach on other areas of the source where those sysctls work without
error. It seems to only happen to custom sysctls that I add. This
happens on multiple machines, but the output in this email is from the
bhyve VM I test in. Any insight as to where my implementation went wrong
would be extremely helpful.

Full Source: ("make" should compile on a bsd system)
https://github.com/ashaferian/sysctls

echo_modevent ()
{
...
        /* working sysctl using default handler sysctl_handle_int */
        SYSCTL_ADD_PROC(&ctx, SYSCTL_CHILDREN(poid), OID_AUTO, "default", CTLTYPE_INT|CTLFLAG_RW, &ret, -1, sysctl_handle_int, "I", "working sysctl");

        /* broken sysctl using my handler */
        SYSCTL_ADD_PROC(&ctx, SYSCTL_CHILDREN(poid), OID_AUTO, "custom",
        CTLTYPE_INT|CTLFLAG_RW, &ret, -1, sysctl_handle, "I", "working
        sysctl");
}

static int
sysctl_handle(SYSCTL_HANDLER_ARGS)
{
        int error;

        /* returns ENOMEM */
        error = sysctl_handle_int(oidp, arg1, arg2, req);
...

In the code above I use the "sysctl_handle" function to handle requests
to the echo.* sysctls I have created. I create two nodes echo.default
which uses the default handler "sysctl_handle_int", and echo.custom
which uses my "sysctl_handle" function. What confuses me so greatly is
that sysctl_handle is just a wrapper that calls sysctl_handle_int and
passes its arguments without changing anything. There should be no
difference because all I do is call the default handler. echo.default works,
while echo.custom does not. Both functions operate on the same
variables and have pretty much the same SYSCLT_ADD_PROC declaration.

My best guess:
I've tried to dig around in the debugger (using kgdb) but haven't been
able to find anything. The only difference I noticed while tracing was
that the "oldlenp" field in the sysctl_args struct passed in from
userland was 0 when calling my handler and 8 when calling the default
handler. I am not very familiar with the sysctl implementation, but this
affected the valid length in the sysctl_req which ended up returning
ENOMEM from sysctl_old_user. You can watch/confirm this change in sysctl_req
using the following dtrace one-liner. Its unclear why changing the
sysctl handler would affect the arguments passed to it.

---------------------------
dtrace -n '*::sysctl_handle_int:entry /execname == "sysctl" / {
print(*args[3]); }'


// sysctl echo.default
  0  27761          sysctl_handle_int:entry struct sysctl_req {                          
    struct thread *td = 0xfffff8000392f580
    int lock = 0x1
    void *oldptr = 0x7fffffffd69c
    size_t oldlen = 0x4
    size_t oldidx = 0
    int (*)() oldfunc = kernel`sysctl_old_user                                          
    void *newptr = 0
    size_t newlen = 0
    size_t newidx = 0
    int (*)() newfunc = kernel`sysctl_new_user                                          
    size_t validlen = 0x4
    int flags = 0
}
  0  27761          sysctl_handle_int:entry struct sysctl_req {                          
    struct thread *td = 0xfffff8000392f580
    int lock = 0x1
    void *oldptr = 0
    size_t oldlen = 0
    size_t oldidx = 0
    int (*)() oldfunc = kernel`sysctl_old_user                                          
    void *newptr = 0
    size_t newlen = 0
    size_t newidx = 0
    int (*)() newfunc = kernel`sysctl_new_user                                          
    size_t validlen = 0
    int flags = 0
}
  0  27761          sysctl_handle_int:entry struct sysctl_req {                          
    struct thread *td = 0xfffff8000392f580
    int lock = 0x1
    void *oldptr = 0x800668000
    size_t oldlen = 0x8
    size_t oldidx = 0
    int (*)() oldfunc = kernel`sysctl_old_user                                          
    void *newptr = 0
    size_t newlen = 0
    size_t newidx = 0
    int (*)() newfunc = kernel`sysctl_new_user                                          
    size_t validlen = 0x8
    int flags = 0
}

// sysctl echo.custom
Custom Sysctl: Error 0
Custom Sysctl: Error 12
CPU     ID                    FUNCTION:NAME
  1  27761          sysctl_handle_int:entry struct sysctl_req {                          
    struct thread *td = 0xfffff8000392f580
    int lock = 0x1
    void *oldptr = 0x7fffffffd69c
    size_t oldlen = 0x4
    size_t oldidx = 0
    int (*)() oldfunc = kernel`sysctl_old_user                                          
    void *newptr = 0
    size_t newlen = 0
    size_t newidx = 0
    int (*)() newfunc = kernel`sysctl_new_user                                          
    size_t validlen = 0x4
    int flags = 0
}
  1  27761          sysctl_handle_int:entry struct sysctl_req {                          
    struct thread *td = 0xfffff8000392f580
    int lock = 0x1
    void *oldptr = 0
    size_t oldlen = 0
    size_t oldidx = 0
    int (*)() oldfunc = kernel`sysctl_old_user                                          
    void *newptr = 0
    size_t newlen = 0
    size_t newidx = 0
    int (*)() newfunc = kernel`sysctl_new_user                                          
    size_t validlen = 0
    int flags = 0
}
  1  27761          sysctl_handle_int:entry struct sysctl_req {                          
    struct thread *td = 0xfffff8000392f580
    int lock = 0x1
    void *oldptr = 0x800667008
    size_t oldlen = 0
    size_t oldidx = 0
    int (*)() oldfunc = kernel`sysctl_old_user                                          
    void *newptr = 0
    size_t newlen = 0
    size_t newidx = 0
    int (*)() newfunc = kernel`sysctl_new_user                                          
    size_t validlen = 0
    int flags = 0
}
---------------------------

I'm really not sure what I've done wrong so any help is greatly
appreciated. Most of the online resources/books that show sysctl
creation seem to do the same thing I did. If there is tricky or
non-obvious behavior that I have missed it would be helpful to note it
for others trying to learn.

If there is anything else I can provide or do please let me know.

Thank you so much for your time!
      Austin Shafer

_________________________________________________________________________
uname -a:
FreeBSD punk-vm 13.0-CURRENT FreeBSD 13.0-CURRENT GENERIC-NODEBUG  amd64
        * uname doesn't list revision but it should be r342141

top memory usage:
...
Mem: 20M Active, 2672K Inact, 143M Wired, 98M Buf, 1789M Free

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: ENOMEM when calling sysctl_handle_int from custom handler

Konstantin Belousov
On Mon, Dec 17, 2018 at 05:34:05PM -0500, Austin Shafer wrote:

> Hi freebsd-hackers,
>
> I've had an issue with creating a custom sysctl handler that I can't
> seem to find an answer for. Using sysctl_handle_int as a handler in
> SYSCTL_ADD_PROC works fine, but calling sysctl_handle_int from a custom
> sysctl handler results in an error.
>
> (sorry for the long message, I tried to be verbose)
>
> Creating the nodes works fine, but SYSCTL_OUT or sysctl_handle_int
> return ENOMEM despite there being plenty of available memory. I based my
> approach on other areas of the source where those sysctls work without
> error. It seems to only happen to custom sysctls that I add. This
> happens on multiple machines, but the output in this email is from the
> bhyve VM I test in. Any insight as to where my implementation went wrong
> would be extremely helpful.
>
> Full Source: ("make" should compile on a bsd system)
> https://github.com/ashaferian/sysctls
>
> echo_modevent ()
> {
> ...
>         /* working sysctl using default handler sysctl_handle_int */
> SYSCTL_ADD_PROC(&ctx, SYSCTL_CHILDREN(poid), OID_AUTO, "default", CTLTYPE_INT|CTLFLAG_RW, &ret, -1, sysctl_handle_int, "I", "working sysctl");
>
> /* broken sysctl using my handler */
> SYSCTL_ADD_PROC(&ctx, SYSCTL_CHILDREN(poid), OID_AUTO, "custom",
> CTLTYPE_INT|CTLFLAG_RW, &ret, -1, sysctl_handle, "I", "working
> sysctl");
> }
>
> static int
> sysctl_handle(SYSCTL_HANDLER_ARGS)
> {
> int error;
>
>         /* returns ENOMEM */
> error = sysctl_handle_int(oidp, arg1, arg2, req);
> ...
>
> In the code above I use the "sysctl_handle" function to handle requests
> to the echo.* sysctls I have created. I create two nodes echo.default
> which uses the default handler "sysctl_handle_int", and echo.custom
> which uses my "sysctl_handle" function. What confuses me so greatly is
> that sysctl_handle is just a wrapper that calls sysctl_handle_int and
> passes its arguments without changing anything. There should be no
> difference because all I do is call the default handler. echo.default works,
> while echo.custom does not. Both functions operate on the same
> variables and have pretty much the same SYSCLT_ADD_PROC declaration.
>
> My best guess:
> I've tried to dig around in the debugger (using kgdb) but haven't been
> able to find anything. The only difference I noticed while tracing was
> that the "oldlenp" field in the sysctl_args struct passed in from
> userland was 0 when calling my handler and 8 when calling the default
> handler. I am not very familiar with the sysctl implementation, but this
> affected the valid length in the sysctl_req which ended up returning
> ENOMEM from sysctl_old_user. You can watch/confirm this change in sysctl_req
> using the following dtrace one-liner. Its unclear why changing the
> sysctl handler would affect the arguments passed to it.
Yes, this is most likely cause for your issue.  ENOMEM does not have
anything with the memory shortage, but with the fact that the oldlen
is too short for int copyout.

Note that for the int-typed oid, old and new len should be 4 (not 0 and
not 8).  Why your userspace code passed such length is the question to
you.

>
> ---------------------------
> dtrace -n '*::sysctl_handle_int:entry /execname == "sysctl" / {
> print(*args[3]); }'
>
>
> // sysctl echo.default
>   0  27761          sysctl_handle_int:entry struct sysctl_req {                          
>     struct thread *td = 0xfffff8000392f580
>     int lock = 0x1
>     void *oldptr = 0x7fffffffd69c
>     size_t oldlen = 0x4
>     size_t oldidx = 0
>     int (*)() oldfunc = kernel`sysctl_old_user                                          
>     void *newptr = 0
>     size_t newlen = 0
>     size_t newidx = 0
>     int (*)() newfunc = kernel`sysctl_new_user                                          
>     size_t validlen = 0x4
>     int flags = 0
> }
>   0  27761          sysctl_handle_int:entry struct sysctl_req {                          
>     struct thread *td = 0xfffff8000392f580
>     int lock = 0x1
>     void *oldptr = 0
>     size_t oldlen = 0
>     size_t oldidx = 0
>     int (*)() oldfunc = kernel`sysctl_old_user                                          
>     void *newptr = 0
>     size_t newlen = 0
>     size_t newidx = 0
>     int (*)() newfunc = kernel`sysctl_new_user                                          
>     size_t validlen = 0
>     int flags = 0
> }
>   0  27761          sysctl_handle_int:entry struct sysctl_req {                          
>     struct thread *td = 0xfffff8000392f580
>     int lock = 0x1
>     void *oldptr = 0x800668000
>     size_t oldlen = 0x8
>     size_t oldidx = 0
>     int (*)() oldfunc = kernel`sysctl_old_user                                          
>     void *newptr = 0
>     size_t newlen = 0
>     size_t newidx = 0
>     int (*)() newfunc = kernel`sysctl_new_user                                          
>     size_t validlen = 0x8
>     int flags = 0
> }
>
> // sysctl echo.custom
> Custom Sysctl: Error 0
> Custom Sysctl: Error 12
> CPU     ID                    FUNCTION:NAME
>   1  27761          sysctl_handle_int:entry struct sysctl_req {                          
>     struct thread *td = 0xfffff8000392f580
>     int lock = 0x1
>     void *oldptr = 0x7fffffffd69c
>     size_t oldlen = 0x4
>     size_t oldidx = 0
>     int (*)() oldfunc = kernel`sysctl_old_user                                          
>     void *newptr = 0
>     size_t newlen = 0
>     size_t newidx = 0
>     int (*)() newfunc = kernel`sysctl_new_user                                          
>     size_t validlen = 0x4
>     int flags = 0
> }
>   1  27761          sysctl_handle_int:entry struct sysctl_req {                          
>     struct thread *td = 0xfffff8000392f580
>     int lock = 0x1
>     void *oldptr = 0
>     size_t oldlen = 0
>     size_t oldidx = 0
>     int (*)() oldfunc = kernel`sysctl_old_user                                          
>     void *newptr = 0
>     size_t newlen = 0
>     size_t newidx = 0
>     int (*)() newfunc = kernel`sysctl_new_user                                          
>     size_t validlen = 0
>     int flags = 0
> }
>   1  27761          sysctl_handle_int:entry struct sysctl_req {                          
>     struct thread *td = 0xfffff8000392f580
>     int lock = 0x1
>     void *oldptr = 0x800667008
>     size_t oldlen = 0
>     size_t oldidx = 0
>     int (*)() oldfunc = kernel`sysctl_old_user                                          
>     void *newptr = 0
>     size_t newlen = 0
>     size_t newidx = 0
>     int (*)() newfunc = kernel`sysctl_new_user                                          
>     size_t validlen = 0
>     int flags = 0
> }
> ---------------------------
>
> I'm really not sure what I've done wrong so any help is greatly
> appreciated. Most of the online resources/books that show sysctl
> creation seem to do the same thing I did. If there is tricky or
> non-obvious behavior that I have missed it would be helpful to note it
> for others trying to learn.
>
> If there is anything else I can provide or do please let me know.
>
> Thank you so much for your time!
>       Austin Shafer
>
> _________________________________________________________________________
> uname -a:
> FreeBSD punk-vm 13.0-CURRENT FreeBSD 13.0-CURRENT GENERIC-NODEBUG  amd64
>         * uname doesn't list revision but it should be r342141
>
> top memory usage:
> ...
> Mem: 20M Active, 2672K Inact, 143M Wired, 98M Buf, 1789M Free
>
> _______________________________________________
> [hidden email] mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "[hidden email]"
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: ENOMEM when calling sysctl_handle_int from custom handler

Austin Shafer
Konstantin Belousov <[hidden email]> writes:

> On Mon, Dec 17, 2018 at 05:34:05PM -0500, Austin Shafer wrote:
>> My best guess:
>> I've tried to dig around in the debugger (using kgdb) but haven't been
>> able to find anything. The only difference I noticed while tracing was
>> that the "oldlenp" field in the sysctl_args struct passed in from
>> userland was 0 when calling my handler and 8 when calling the default
>> handler. I am not very familiar with the sysctl implementation, but this
>> affected the valid length in the sysctl_req which ended up returning
>> ENOMEM from sysctl_old_user. You can watch/confirm this change in sysctl_req
>> using the following dtrace one-liner. Its unclear why changing the
>> sysctl handler would affect the arguments passed to it.
> Yes, this is most likely cause for your issue.  ENOMEM does not have
> anything with the memory shortage, but with the fact that the oldlen
> is too short for int copyout.
>
> Note that for the int-typed oid, old and new len should be 4 (not 0 and
> not 8).  Why your userspace code passed such length is the question to
> you.
>

Thanks for the reply! I'm just using the sysctl(8) utility in the
examples above so the valid length passing from userspace is not done by
me. Based on what you said it seems to me that sysctl(8) is not passing
the correct values then? This would be strange since all other sysctls
work using sysctl(8). I'm more than happy to dig deep and debug
sysctl(8) to see if it is the culprit. I had assumed it was working fine
and that the error was on my end in the kernel module.

You are right about the userspace passed values being whats causing my
problems though. I wrote a short program to call sysctlbyname (with the
correct length as you specified above) and now I can set the value of the
sysctl:

// read_sysctl.c
int
main (int argc, char *argv[])
{
        int error, old, new = 3;
        size_t size;

        size = sizeof(int);
...
        error = sysctlbyname(argv[1], &old, &size, &new, size);
...
}

// dtrace -n '*::sysctl_handle_int:entry /execname == "read_sysctl" / { print(*args[3]); }'
CPU     ID                    FUNCTION:NAME
  2  27761          sysctl_handle_int:entry struct sysctl_req {
    struct thread *td = 0xfffff80003b38580
    int lock = 0x1
    void *oldptr = 0x7fffffffd71c
    size_t oldlen = 0x4
    size_t oldidx = 0
    int (*)() oldfunc = kernel`sysctl_old_user
    void *newptr = 0
    size_t newlen = 0
    size_t newidx = 0
    int (*)() newfunc = kernel`sysctl_new_user
    size_t validlen = 0x4
    int flags = 0
}
  2  27761          sysctl_handle_int:entry struct sysctl_req {
    struct thread *td = 0xfffff80003b38580
    int lock = 0x1
    void *oldptr = 0x7fffffffeb28
    size_t oldlen = 0x4
    size_t oldidx = 0
    int (*)() oldfunc = kernel`sysctl_old_user
    void *newptr = 0x7fffffffeb24
    size_t newlen = 0x4
    size_t newidx = 0
    int (*)() newfunc = kernel`sysctl_new_user
    size_t validlen = 0x4
    int flags = 0
}

Also out of curiosity, the dtrace probes show multiple sysctl_handle_int
evaluations. Is this for each root and sub-node in the sysctl tree?
Thats what it looks like to me but I'd love to know for sure.

Thanks for all the help!
       Austin Shafer
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: ENOMEM when calling sysctl_handle_int from custom handler

Ryan Stone-2
In reply to this post by Austin Shafer
You should not pass arg1 and arg2 to sysctl_handle_int().  You instead
need to pass a pointer to a local value containing the value you want
to return to the sysctl caller.  After sysctl_handle_int returns, if
it returned 0 and req->newptr is non-NULL, then the integer value will
contain the new value that was passed to you from userland.  You want
something that looks like this:

int
my_sysctl_handler(SYSCTL_HANDLER_ARGS)
{
     int val, error;

     val = 5; /* Or whatever value you want to return from userland. */

     error = sysctl_handle_int(oidp, &val, 0, req);
     if (error != 0 || req->newptr == NULL)
        return (error);

    /* val contains the value set by the caller, so do something
interesting with it here. */

    return (0);
}
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: ENOMEM when calling sysctl_handle_int from custom handler

Austin Shafer
Ryan Stone <[hidden email]> writes:

> You should not pass arg1 and arg2 to sysctl_handle_int().  You instead
> need to pass a pointer to a local value containing the value you want
> to return to the sysctl caller.  After sysctl_handle_int returns, if
> it returned 0 and req->newptr is non-NULL, then the integer value will
> contain the new value that was passed to you from userland.  You want
> something that looks like this:
>
> int
> my_sysctl_handler(SYSCTL_HANDLER_ARGS)
> {
>      int val, error;
>
>      val = 5; /* Or whatever value you want to return from userland. */
>
>      error = sysctl_handle_int(oidp, &val, 0, req);
>      if (error != 0 || req->newptr == NULL)
>         return (error);
>
>     /* val contains the value set by the caller, so do something
> interesting with it here. */
>
>     return (0);
> }

Hmm yup this seems to be problem. I had originally tried to just pass
the handler args directly through to sysctl_handle_int to make sure I
was creating the nodes correctly, since I knew sysctl_handle_int
worked.

Thanks again for all the help on these beginner questions!
       Austin Shafer
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"