diff mbox series

[7/9] bsd-user: do_freebsd_sysctl helper for sysctl(2)

Message ID 20230210231829.39476-8-imp@bsdimp.com (mailing list archive)
State New, archived
Headers show
Series 2023 Q1 bsd-user upstreaming: bugfixes and sysctl | expand

Commit Message

Warner Losh Feb. 10, 2023, 11:18 p.m. UTC 
From: Kyle Evans <kevans@FreeBSD.org>

Implement the wrapper function for sysctl(2). This puts the oid
arguments into a standard form and calls the common
do_freebsd_sysctl_oid.

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Co-Authored-by: Juergen Lock <nox@jelal.kn-bremen.de>
Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
Co-Authored-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-sys.c     | 50 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  4 +++
 2 files changed, 54 insertions(+)

Comments

Richard Henderson Feb. 11, 2023, 11:09 p.m. UTC | #1 
On 2/10/23 13:18, Warner Losh wrote:
> +abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
> +        abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen)
> +{
> +    abi_long ret;
> +    void *hnamep, *holdp = NULL, *hnewp = NULL;
> +    size_t holdlen;
> +    abi_ulong oldlen = 0;
> +    int32_t *snamep = g_malloc(sizeof(int32_t) * namelen), *p, *q, i;
> +
> +    if (oldlenp) {
> +        if (get_user_ual(oldlen, oldlenp)) {
> +            return -TARGET_EFAULT;
> +        }
> +    }

You need to check for write early.  Either access_ok, or lock_user.

> +    for (p = hnamep, q = snamep, i = 0; i < namelen; p++, i++) {
> +        *q++ = tswap32(*p);
> +    }

Why the inconsistent increments?

> +    unlock_user(holdp, oldp, holdlen);

Usually we don't want writeback on error.


r~
Warner Losh Feb. 12, 2023, 5:53 p.m. UTC | #2 
On Sat, Feb 11, 2023 at 4:09 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 2/10/23 13:18, Warner Losh wrote:
> > +abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t
> namelen,
> > +        abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong
> newlen)
> > +{
> > +    abi_long ret;
> > +    void *hnamep, *holdp = NULL, *hnewp = NULL;
> > +    size_t holdlen;
> > +    abi_ulong oldlen = 0;
> > +    int32_t *snamep = g_malloc(sizeof(int32_t) * namelen), *p, *q, i;
> > +
> > +    if (oldlenp) {
> > +        if (get_user_ual(oldlen, oldlenp)) {
> > +            return -TARGET_EFAULT;
> > +        }
> > +    }
>
> You need to check for write early.  Either access_ok, or lock_user.
>
> > +    for (p = hnamep, q = snamep, i = 0; i < namelen; p++, i++) {
> > +        *q++ = tswap32(*p);
> > +    }
>
> Why the inconsistent increments?
>

no reason... Fixed.


> > +    unlock_user(holdp, oldp, holdlen);
>
> Usually we don't want writeback on error.
>

Indeed. Fixed as well.. in the other fixes for error handling.

Warner


>
> r~
>
diff mbox series

Patch

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index a8fb29f36b7..13736936e5f 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -345,6 +345,56 @@  out:
     return ret;
 }
 
+abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
+        abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen)
+{
+    abi_long ret;
+    void *hnamep, *holdp = NULL, *hnewp = NULL;
+    size_t holdlen;
+    abi_ulong oldlen = 0;
+    int32_t *snamep = g_malloc(sizeof(int32_t) * namelen), *p, *q, i;
+
+    if (oldlenp) {
+        if (get_user_ual(oldlen, oldlenp)) {
+            return -TARGET_EFAULT;
+        }
+    }
+    hnamep = lock_user(VERIFY_READ, namep, namelen, 1);
+    if (hnamep == NULL) {
+        return -TARGET_EFAULT;
+    }
+    if (newp) {
+        hnewp = lock_user(VERIFY_READ, newp, newlen, 1);
+        if (hnewp == NULL) {
+            return -TARGET_EFAULT;
+        }
+    }
+    if (oldp) {
+        holdp = lock_user(VERIFY_WRITE, oldp, oldlen, 0);
+        if (holdp == NULL) {
+            return -TARGET_EFAULT;
+        }
+    }
+    holdlen = oldlen;
+    for (p = hnamep, q = snamep, i = 0; i < namelen; p++, i++) {
+        *q++ = tswap32(*p);
+    }
+
+    ret = do_freebsd_sysctl_oid(env, snamep, namelen, holdp, &holdlen, hnewp,
+        newlen);
+
+    if (oldlenp) {
+        put_user_ual(holdlen, oldlenp);
+    }
+    unlock_user(hnamep, namep, 0);
+    unlock_user(holdp, oldp, holdlen);
+    if (hnewp) {
+        unlock_user(hnewp, newp, 0);
+    }
+    g_free(snamep);
+    return ret;
+}
+
 /* sysarch() is architecture dependent. */
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
 {
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index e00997a818c..20ab3d4d9a1 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -494,6 +494,10 @@  static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         /*
          * sys{ctl, arch, call}
          */
+    case TARGET_FREEBSD_NR___sysctl: /* sysctl(3) */
+        ret = do_freebsd_sysctl(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6);
+        break;
+
     case TARGET_FREEBSD_NR_sysarch: /* sysarch(2) */
         ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
         break;