[PATCH v4 18/24] bsd-user: Add do_bsd___semctl implementation

Warner Losh posted 24 patches 1 month, 2 weeks ago
Maintainers: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Riku Voipio <riku.voipio@iki.fi>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
[PATCH v4 18/24] bsd-user: Add do_bsd___semctl implementation
Posted by Warner Losh 1 month, 2 weeks ago
From: Stacey Son <sson@FreeBSD.org>

Add implementation of __semctl(2) syscall for System V semaphore control
operations. Handles command translation, endianness conversion for GETVAL/
SETVAL, and array/structure conversions for GETALL/SETALL/IPC_STAT/IPC_SET.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Cc: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/bsd-misc.h | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/bsd-user/bsd-misc.h b/bsd-user/bsd-misc.h
index e1e552b58f..72b3db0b74 100644
--- a/bsd-user/bsd-misc.h
+++ b/bsd-user/bsd-misc.h
@@ -99,6 +99,117 @@ static inline abi_long do_bsd_semop(int semid, abi_long ptr, unsigned nsops)
     return semop(semid, sops, nsops);
 }
 
+/* __semctl(2) */
+static inline abi_long do_bsd___semctl(int semid, int semnum, int target_cmd,
+                                       abi_ptr un_ptr)
+{
+    void *target_un;
+    union semun arg;
+    struct semid_ds dsarg;
+    unsigned short *array = NULL;
+    int host_cmd;
+    abi_long ret = 0;
+    abi_long err;
+    abi_ulong target_array, target_buffer;
+
+    switch (target_cmd) {
+    case TARGET_GETVAL:
+        host_cmd = GETVAL;
+        break;
+
+    case TARGET_SETVAL:
+        host_cmd = SETVAL;
+        break;
+
+    case TARGET_GETALL:
+        host_cmd = GETALL;
+        break;
+
+    case TARGET_SETALL:
+        host_cmd = SETALL;
+        break;
+
+    case TARGET_IPC_STAT:
+        host_cmd = IPC_STAT;
+        break;
+
+    case TARGET_IPC_SET:
+        host_cmd = IPC_SET;
+        break;
+
+    case TARGET_IPC_RMID:
+        host_cmd = IPC_RMID;
+        break;
+
+    case TARGET_GETPID:
+        host_cmd = GETPID;
+        break;
+
+    case TARGET_GETNCNT:
+        host_cmd = GETNCNT;
+        break;
+
+    case TARGET_GETZCNT:
+        host_cmd = GETZCNT;
+        break;
+
+    default:
+        return -TARGET_EINVAL;
+    }
+
+    /*
+     * Unlike Linux and the semctl system call, we take a pointer
+     * to the union arg here.
+     */
+    target_un = lock_user(VERIFY_READ, un_ptr, sizeof(union target_semun), 1);
+
+    switch (host_cmd) {
+    case GETVAL:
+    case SETVAL:
+        __get_user(arg.val, (abi_int *)target_un);
+        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
+        break;
+
+    case GETALL:
+    case SETALL:
+        __get_user(target_array, (abi_ulong *)target_un);
+        err = target_to_host_semarray(semid, &array, target_array);
+        if (is_error(err)) {
+            goto out;
+        }
+        arg.array = array;
+        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
+        err = host_to_target_semarray(semid, target_array, &array);
+        break;
+
+    case IPC_STAT:
+    case IPC_SET:
+        __get_user(target_buffer, (abi_ulong *)target_un);
+        err = target_to_host_semid_ds(&dsarg, target_buffer);
+        if (is_error(err)) {
+            goto out;
+        }
+        arg.buf = &dsarg;
+        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
+        err = host_to_target_semid_ds(target_buffer, &dsarg);
+        break;
+
+    case IPC_RMID:
+    case GETPID:
+    case GETNCNT:
+    case GETZCNT:
+        ret = get_errno(semctl(semid, semnum, host_cmd, NULL));
+        break;
+
+    default:
+        ret = -TARGET_EINVAL;
+        break;
+    }
+out:
+    unlock_user(target_un, un_ptr, 1);
+    return ret;
+}
+
 /* getdtablesize(2) */
 static inline abi_long do_bsd_getdtablesize(void)
 {

-- 
2.52.0
Re: [PATCH v4 18/24] bsd-user: Add do_bsd___semctl implementation
Posted by Pierrick Bouvier 1 month, 2 weeks ago
On 2/24/26 6:40 AM, Warner Losh wrote:
> From: Stacey Son <sson@FreeBSD.org>
> 
> Add implementation of __semctl(2) syscall for System V semaphore control
> operations. Handles command translation, endianness conversion for GETVAL/
> SETVAL, and array/structure conversions for GETALL/SETALL/IPC_STAT/IPC_SET.
> 
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Cc: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/bsd-misc.h | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 111 insertions(+)
> 
> diff --git a/bsd-user/bsd-misc.h b/bsd-user/bsd-misc.h
> index e1e552b58f..72b3db0b74 100644
> --- a/bsd-user/bsd-misc.h
> +++ b/bsd-user/bsd-misc.h
> @@ -99,6 +99,117 @@ static inline abi_long do_bsd_semop(int semid, abi_long ptr, unsigned nsops)
>       return semop(semid, sops, nsops);
>   }
>   
> +/* __semctl(2) */
> +static inline abi_long do_bsd___semctl(int semid, int semnum, int target_cmd,
> +                                       abi_ptr un_ptr)
> +{
> +    void *target_un;
> +    union semun arg;
> +    struct semid_ds dsarg;
> +    unsigned short *array = NULL;
> +    int host_cmd;
> +    abi_long ret = 0;
> +    abi_long err;
> +    abi_ulong target_array, target_buffer;
> +
> +    switch (target_cmd) {
> +    case TARGET_GETVAL:
> +        host_cmd = GETVAL;
> +        break;
> +
> +    case TARGET_SETVAL:
> +        host_cmd = SETVAL;
> +        break;
> +
> +    case TARGET_GETALL:
> +        host_cmd = GETALL;
> +        break;
> +
> +    case TARGET_SETALL:
> +        host_cmd = SETALL;
> +        break;
> +
> +    case TARGET_IPC_STAT:
> +        host_cmd = IPC_STAT;
> +        break;
> +
> +    case TARGET_IPC_SET:
> +        host_cmd = IPC_SET;
> +        break;
> +
> +    case TARGET_IPC_RMID:
> +        host_cmd = IPC_RMID;
> +        break;
> +
> +    case TARGET_GETPID:
> +        host_cmd = GETPID;
> +        break;
> +
> +    case TARGET_GETNCNT:
> +        host_cmd = GETNCNT;
> +        break;
> +
> +    case TARGET_GETZCNT:
> +        host_cmd = GETZCNT;
> +        break;
> +
> +    default:
> +        return -TARGET_EINVAL;
> +    }
> +
> +    /*
> +     * Unlike Linux and the semctl system call, we take a pointer
> +     * to the union arg here.
> +     */
> +    target_un = lock_user(VERIFY_READ, un_ptr, sizeof(union target_semun), 1);
> +
> +    switch (host_cmd) {
> +    case GETVAL:
> +    case SETVAL:
> +        __get_user(arg.val, (abi_int *)target_un);
> +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> +        break;
> +
> +    case GETALL:
> +    case SETALL:
> +        __get_user(target_array, (abi_ulong *)target_un);
> +        err = target_to_host_semarray(semid, &array, target_array);
> +        if (is_error(err)) {
> +            goto out;
> +        }

Seems like ret should be set to an error also, else we'll silently 
return default value 0. I'm not sure which error code we should return 
(EINVAL?)

> +        arg.array = array;
> +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> +        err = host_to_target_semarray(semid, target_array, &array);
> +        break;
> +
> +    case IPC_STAT:
> +    case IPC_SET:
> +        __get_user(target_buffer, (abi_ulong *)target_un);
> +        err = target_to_host_semid_ds(&dsarg, target_buffer);
> +        if (is_error(err)) {
> +            goto out;
> +        }
> +        arg.buf = &dsarg;
> +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> +        err = host_to_target_semid_ds(target_buffer, &dsarg);
> +        break;
> +
> +    case IPC_RMID:
> +    case GETPID:
> +    case GETNCNT:
> +    case GETZCNT:
> +        ret = get_errno(semctl(semid, semnum, host_cmd, NULL));
> +        break;
> +
> +    default:
> +        ret = -TARGET_EINVAL;
> +        break;
> +    }
> +out:
> +    unlock_user(target_un, un_ptr, 1);
> +    return ret;
> +}
> +
>   /* getdtablesize(2) */
>   static inline abi_long do_bsd_getdtablesize(void)
>   {
> 

With that,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Feel free to add the fix and pull directly without reposting so you can 
save some time before softfreeze.

Regards,
Pierrick
Re: [PATCH v4 18/24] bsd-user: Add do_bsd___semctl implementation
Posted by Warner Losh 1 month, 2 weeks ago
On Thu, Feb 26, 2026 at 12:56 PM Pierrick Bouvier <
pierrick.bouvier@linaro.org> wrote:

> On 2/24/26 6:40 AM, Warner Losh wrote:
> > From: Stacey Son <sson@FreeBSD.org>
> >
> > Add implementation of __semctl(2) syscall for System V semaphore control
> > operations. Handles command translation, endianness conversion for
> GETVAL/
> > SETVAL, and array/structure conversions for
> GETALL/SETALL/IPC_STAT/IPC_SET.
> >
> > Signed-off-by: Stacey Son <sson@FreeBSD.org>
> > Cc: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >   bsd-user/bsd-misc.h | 111
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 111 insertions(+)
> >
> > diff --git a/bsd-user/bsd-misc.h b/bsd-user/bsd-misc.h
> > index e1e552b58f..72b3db0b74 100644
> > --- a/bsd-user/bsd-misc.h
> > +++ b/bsd-user/bsd-misc.h
> > @@ -99,6 +99,117 @@ static inline abi_long do_bsd_semop(int semid,
> abi_long ptr, unsigned nsops)
> >       return semop(semid, sops, nsops);
> >   }
> >
> > +/* __semctl(2) */
> > +static inline abi_long do_bsd___semctl(int semid, int semnum, int
> target_cmd,
> > +                                       abi_ptr un_ptr)
> > +{
> > +    void *target_un;
> > +    union semun arg;
> > +    struct semid_ds dsarg;
> > +    unsigned short *array = NULL;
> > +    int host_cmd;
> > +    abi_long ret = 0;
> > +    abi_long err;
> > +    abi_ulong target_array, target_buffer;
> > +
> > +    switch (target_cmd) {
> > +    case TARGET_GETVAL:
> > +        host_cmd = GETVAL;
> > +        break;
> > +
> > +    case TARGET_SETVAL:
> > +        host_cmd = SETVAL;
> > +        break;
> > +
> > +    case TARGET_GETALL:
> > +        host_cmd = GETALL;
> > +        break;
> > +
> > +    case TARGET_SETALL:
> > +        host_cmd = SETALL;
> > +        break;
> > +
> > +    case TARGET_IPC_STAT:
> > +        host_cmd = IPC_STAT;
> > +        break;
> > +
> > +    case TARGET_IPC_SET:
> > +        host_cmd = IPC_SET;
> > +        break;
> > +
> > +    case TARGET_IPC_RMID:
> > +        host_cmd = IPC_RMID;
> > +        break;
> > +
> > +    case TARGET_GETPID:
> > +        host_cmd = GETPID;
> > +        break;
> > +
> > +    case TARGET_GETNCNT:
> > +        host_cmd = GETNCNT;
> > +        break;
> > +
> > +    case TARGET_GETZCNT:
> > +        host_cmd = GETZCNT;
> > +        break;
> > +
> > +    default:
> > +        return -TARGET_EINVAL;
> > +    }
> > +
> > +    /*
> > +     * Unlike Linux and the semctl system call, we take a pointer
> > +     * to the union arg here.
> > +     */
> > +    target_un = lock_user(VERIFY_READ, un_ptr, sizeof(union
> target_semun), 1);
> > +
> > +    switch (host_cmd) {
> > +    case GETVAL:
> > +    case SETVAL:
> > +        __get_user(arg.val, (abi_int *)target_un);
> > +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> > +        break;
> > +
> > +    case GETALL:
> > +    case SETALL:
> > +        __get_user(target_array, (abi_ulong *)target_un);
> > +        err = target_to_host_semarray(semid, &array, target_array);
> > +        if (is_error(err)) {
> > +            goto out;
> > +        }
>
> Seems like ret should be set to an error also, else we'll silently
> return default value 0. I'm not sure which error code we should return
> (EINVAL?)
>

Ah yes. I'll look at it and fix it.


> > +        arg.array = array;
> > +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> > +        err = host_to_target_semarray(semid, target_array, &array);
>

And this is starting to look wrong too. I'll fix that.


> > +        break;
> > +
> > +    case IPC_STAT:
> > +    case IPC_SET:
> > +        __get_user(target_buffer, (abi_ulong *)target_un);
> > +        err = target_to_host_semid_ds(&dsarg, target_buffer);
> > +        if (is_error(err)) {
> > +            goto out;
> > +        }
> > +        arg.buf = &dsarg;
> > +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> > +        err = host_to_target_semid_ds(target_buffer, &dsarg);
> > +        break;
> > +
> > +    case IPC_RMID:
> > +    case GETPID:
> > +    case GETNCNT:
> > +    case GETZCNT:
> > +        ret = get_errno(semctl(semid, semnum, host_cmd, NULL));
> > +        break;
> > +
> > +    default:
> > +        ret = -TARGET_EINVAL;
> > +        break;
> > +    }
> > +out:
> > +    unlock_user(target_un, un_ptr, 1);
> > +    return ret;
> > +}
> > +
> >   /* getdtablesize(2) */
> >   static inline abi_long do_bsd_getdtablesize(void)
> >   {
> >
>
> With that,
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>
> Feel free to add the fix and pull directly without reposting so you can
> save some time before softfreeze.
>

Thanks! Most of these errors are rare, but still gotta get them right.

Warner
Re: [PATCH v4 18/24] bsd-user: Add do_bsd___semctl implementation
Posted by Warner Losh 1 month, 2 weeks ago
PING?

I'm planning on sending a pull request with this in it Monday March 2nd
unless anybody objects.

Thanks!

On Tue, Feb 24, 2026 at 7:41 AM Warner Losh <imp@bsdimp.com> wrote:

> From: Stacey Son <sson@FreeBSD.org>
>
> Add implementation of __semctl(2) syscall for System V semaphore control
> operations. Handles command translation, endianness conversion for GETVAL/
> SETVAL, and array/structure conversions for GETALL/SETALL/IPC_STAT/IPC_SET.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Cc: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/bsd-misc.h | 111
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 111 insertions(+)
>
> diff --git a/bsd-user/bsd-misc.h b/bsd-user/bsd-misc.h
> index e1e552b58f..72b3db0b74 100644
> --- a/bsd-user/bsd-misc.h
> +++ b/bsd-user/bsd-misc.h
> @@ -99,6 +99,117 @@ static inline abi_long do_bsd_semop(int semid,
> abi_long ptr, unsigned nsops)
>      return semop(semid, sops, nsops);
>  }
>
> +/* __semctl(2) */
> +static inline abi_long do_bsd___semctl(int semid, int semnum, int
> target_cmd,
> +                                       abi_ptr un_ptr)
> +{
> +    void *target_un;
> +    union semun arg;
> +    struct semid_ds dsarg;
> +    unsigned short *array = NULL;
> +    int host_cmd;
> +    abi_long ret = 0;
> +    abi_long err;
> +    abi_ulong target_array, target_buffer;
> +
> +    switch (target_cmd) {
> +    case TARGET_GETVAL:
> +        host_cmd = GETVAL;
> +        break;
> +
> +    case TARGET_SETVAL:
> +        host_cmd = SETVAL;
> +        break;
> +
> +    case TARGET_GETALL:
> +        host_cmd = GETALL;
> +        break;
> +
> +    case TARGET_SETALL:
> +        host_cmd = SETALL;
> +        break;
> +
> +    case TARGET_IPC_STAT:
> +        host_cmd = IPC_STAT;
> +        break;
> +
> +    case TARGET_IPC_SET:
> +        host_cmd = IPC_SET;
> +        break;
> +
> +    case TARGET_IPC_RMID:
> +        host_cmd = IPC_RMID;
> +        break;
> +
> +    case TARGET_GETPID:
> +        host_cmd = GETPID;
> +        break;
> +
> +    case TARGET_GETNCNT:
> +        host_cmd = GETNCNT;
> +        break;
> +
> +    case TARGET_GETZCNT:
> +        host_cmd = GETZCNT;
> +        break;
> +
> +    default:
> +        return -TARGET_EINVAL;
> +    }
> +
> +    /*
> +     * Unlike Linux and the semctl system call, we take a pointer
> +     * to the union arg here.
> +     */
> +    target_un = lock_user(VERIFY_READ, un_ptr, sizeof(union
> target_semun), 1);
> +
> +    switch (host_cmd) {
> +    case GETVAL:
> +    case SETVAL:
> +        __get_user(arg.val, (abi_int *)target_un);
> +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> +        break;
> +
> +    case GETALL:
> +    case SETALL:
> +        __get_user(target_array, (abi_ulong *)target_un);
> +        err = target_to_host_semarray(semid, &array, target_array);
> +        if (is_error(err)) {
> +            goto out;
> +        }
> +        arg.array = array;
> +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> +        err = host_to_target_semarray(semid, target_array, &array);
> +        break;
> +
> +    case IPC_STAT:
> +    case IPC_SET:
> +        __get_user(target_buffer, (abi_ulong *)target_un);
> +        err = target_to_host_semid_ds(&dsarg, target_buffer);
> +        if (is_error(err)) {
> +            goto out;
> +        }
> +        arg.buf = &dsarg;
> +        ret = get_errno(semctl(semid, semnum, host_cmd, arg));
> +        err = host_to_target_semid_ds(target_buffer, &dsarg);
> +        break;
> +
> +    case IPC_RMID:
> +    case GETPID:
> +    case GETNCNT:
> +    case GETZCNT:
> +        ret = get_errno(semctl(semid, semnum, host_cmd, NULL));
> +        break;
> +
> +    default:
> +        ret = -TARGET_EINVAL;
> +        break;
> +    }
> +out:
> +    unlock_user(target_un, un_ptr, 1);
> +    return ret;
> +}
> +
>  /* getdtablesize(2) */
>  static inline abi_long do_bsd_getdtablesize(void)
>  {
>
> --
> 2.52.0
>
>