All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] plugins: fix syscall filter return value type
@ 2026-06-12  5:56 Ziyang Zhang
  2026-06-12  5:56 ` [PATCH 1/1] plugins: use int64_t for the syscall filter return value Ziyang Zhang
  2026-06-12  7:59 ` [PATCH 0/1] plugins: fix syscall filter return value type Ziyang Zhang
  0 siblings, 2 replies; 5+ messages in thread
From: Ziyang Zhang @ 2026-06-12  5:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Riku Voipio, Laurent Vivier, Alex Bennee, Alexandre Iooss,
	Mahmoud Mandour, Pierrick Bouvier, Richard Henderson, Zhengwei Qi,
	Yun Wang, Mingyuan Xia, Kailiang Xu, Ziyang Zhang

Hi,

The syscall filter plugin interface was introduced by me earlier. Its
sysret value is declared as uint64_t, but it is semantically signed --
negative values encode errno codes -- so it should be int64_t. This
also makes it consistent with the ret parameter of
qemu_plugin_vcpu_syscall_ret(), which is already int64_t.

I did not bump QEMU_PLUGIN_VERSION: this is only a signedness fix, and
a version bump is better left for the next significant plugin API
update.

Thanks for your review.

Ziyang Zhang (1):
  plugins: use int64_t for the syscall filter return value

 include/plugins/qemu-plugin.h | 2 +-
 include/qemu/plugin.h         | 4 ++--
 linux-user/syscall.c          | 2 +-
 plugins/core.c                | 2 +-
 tests/tcg/plugins/setpc.c     | 2 +-
 tests/tcg/plugins/syscall.c   | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] plugins: use int64_t for the syscall filter return value
  2026-06-12  5:56 [PATCH 0/1] plugins: fix syscall filter return value type Ziyang Zhang
@ 2026-06-12  5:56 ` Ziyang Zhang
  2026-06-12 10:43   ` Alex Bennée
  2026-06-12  7:59 ` [PATCH 0/1] plugins: fix syscall filter return value type Ziyang Zhang
  1 sibling, 1 reply; 5+ messages in thread
From: Ziyang Zhang @ 2026-06-12  5:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Riku Voipio, Laurent Vivier, Alex Bennee, Alexandre Iooss,
	Mahmoud Mandour, Pierrick Bouvier, Richard Henderson, Zhengwei Qi,
	Yun Wang, Mingyuan Xia, Kailiang Xu, Ziyang Zhang

The syscall return value passed back through the syscall filter
callback is semantically signed: negative values encode errno codes.
Declaring the sysret pointer as uint64_t * is therefore misleading and
forces callers to launder the value through an unsigned temporary.

Change the sysret pointer to int64_t * across the public plugin API
typedef (qemu_plugin_vcpu_syscall_filter_cb_t), the internal
qemu_plugin_vcpu_syscall_filter() prototypes and stub, its
implementation in plugins/core.c, the linux-user caller, and the
in-tree example plugins.

Signed-off-by: Ziyang Zhang <functioner@sjtu.edu.cn>
---
 include/plugins/qemu-plugin.h | 2 +-
 include/qemu/plugin.h         | 4 ++--
 linux-user/syscall.c          | 2 +-
 plugins/core.c                | 2 +-
 tests/tcg/plugins/setpc.c     | 2 +-
 tests/tcg/plugins/syscall.c   | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
index 4eb1d2cd85..8eb10b1f48 100644
--- a/include/plugins/qemu-plugin.h
+++ b/include/plugins/qemu-plugin.h
@@ -870,7 +870,7 @@ typedef bool
                                         int64_t num, uint64_t a1, uint64_t a2,
                                         uint64_t a3, uint64_t a4, uint64_t a5,
                                         uint64_t a6, uint64_t a7, uint64_t a8,
-                                        uint64_t *sysret);
+                                        int64_t *sysret);
 
 /**
  * typedef qemu_plugin_vcpu_syscall_ret_cb_t - vCPU syscall return callback
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index ddd77bd82c..1ce4b281c1 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -174,7 +174,7 @@ bool
 qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
                                 uint64_t a2, uint64_t a3, uint64_t a4,
                                 uint64_t a5, uint64_t a6, uint64_t a7,
-                                uint64_t a8, uint64_t *sysret);
+                                uint64_t a8, int64_t *sysret);
 
 void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
                              uint64_t value_low,
@@ -290,7 +290,7 @@ static inline bool
 qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
                                 uint64_t a2, uint64_t a3, uint64_t a4,
                                 uint64_t a5, uint64_t a6, uint64_t a7,
-                                uint64_t a8, uint64_t *sysret)
+                                uint64_t a8, int64_t *sysret)
 {
     return false;
 }
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f4b74ad350..63c0a5f8f3 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -14378,7 +14378,7 @@ static bool send_through_syscall_filters(CPUState *cpu, int num,
                                          abi_long arg7, abi_long arg8,
                                          abi_long *sysret)
 {
-    uint64_t sysret64 = 0;
+    int64_t sysret64 = 0;
     bool filtered = qemu_plugin_vcpu_syscall_filter(cpu, num, arg1, arg2,
                                                     arg3, arg4, arg5, arg6,
                                                     arg7, arg8, &sysret64);
diff --git a/plugins/core.c b/plugins/core.c
index 2324bbffa3..58f293462a 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -596,7 +596,7 @@ bool
 qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
                                 uint64_t a2, uint64_t a3, uint64_t a4,
                                 uint64_t a5, uint64_t a6, uint64_t a7,
-                                uint64_t a8, uint64_t *sysret)
+                                uint64_t a8, int64_t *sysret)
 {
     struct qemu_plugin_cb *cb, *next;
     enum qemu_plugin_event ev = QEMU_PLUGIN_EV_VCPU_SYSCALL_FILTER;
diff --git a/tests/tcg/plugins/setpc.c b/tests/tcg/plugins/setpc.c
index 8f2d025e24..23862eaaf0 100644
--- a/tests/tcg/plugins/setpc.c
+++ b/tests/tcg/plugins/setpc.c
@@ -27,7 +27,7 @@ static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
                                 int64_t num, uint64_t a1, uint64_t a2,
                                 uint64_t a3, uint64_t a4, uint64_t a5,
                                 uint64_t a6, uint64_t a7, uint64_t a8,
-                                uint64_t *sysret)
+                                int64_t *sysret)
 {
     if (num == MAGIC_SYSCALL) {
         if (a1 == SETPC) {
diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
index 5658f83087..76d52b98aa 100644
--- a/tests/tcg/plugins/syscall.c
+++ b/tests/tcg/plugins/syscall.c
@@ -174,7 +174,7 @@ static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
                                 int64_t num, uint64_t a1, uint64_t a2,
                                 uint64_t a3, uint64_t a4, uint64_t a5,
                                 uint64_t a6, uint64_t a7, uint64_t a8,
-                                uint64_t *sysret)
+                                int64_t *sysret)
 {
     /* Special syscall to test the filter functionality. */
     if (num == 4096 && a1 == 0x66CCFF) {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/1] plugins: fix syscall filter return value type
  2026-06-12  5:56 [PATCH 0/1] plugins: fix syscall filter return value type Ziyang Zhang
  2026-06-12  5:56 ` [PATCH 1/1] plugins: use int64_t for the syscall filter return value Ziyang Zhang
@ 2026-06-12  7:59 ` Ziyang Zhang
  1 sibling, 0 replies; 5+ messages in thread
From: Ziyang Zhang @ 2026-06-12  7:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Riku Voipio, Laurent Vivier, Alex Bennee, Alexandre Iooss,
	Mahmoud Mandour, Pierrick Bouvier, Richard Henderson, Zhengwei Qi,
	Yun Wang, Mingyuan Xia, Kailiang Xu

On Fri, 12 Jun 2026 13:56:41 +0800, Ziyang Zhang wrote:
> Hi,
> 
> The syscall filter plugin interface was introduced by me earlier.

Syscall filter PATCH reference:
https://lore.kernel.org/qemu-devel/20251214144620.179282-1-functioner@sjtu.edu.cn/


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] plugins: use int64_t for the syscall filter return value
  2026-06-12  5:56 ` [PATCH 1/1] plugins: use int64_t for the syscall filter return value Ziyang Zhang
@ 2026-06-12 10:43   ` Alex Bennée
  2026-06-12 12:48     ` Ziyang Zhang
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Bennée @ 2026-06-12 10:43 UTC (permalink / raw)
  To: Ziyang Zhang
  Cc: qemu-devel, Riku Voipio, Laurent Vivier, Alexandre Iooss,
	Mahmoud Mandour, Pierrick Bouvier, Richard Henderson, Zhengwei Qi,
	Yun Wang, Mingyuan Xia, Kailiang Xu

Ziyang Zhang <functioner@sjtu.edu.cn> writes:

> The syscall return value passed back through the syscall filter
> callback is semantically signed: negative values encode errno codes.
> Declaring the sysret pointer as uint64_t * is therefore misleading and
> forces callers to launder the value through an unsigned temporary.
>
> Change the sysret pointer to int64_t * across the public plugin API
> typedef (qemu_plugin_vcpu_syscall_filter_cb_t), the internal
> qemu_plugin_vcpu_syscall_filter() prototypes and stub, its
> implementation in plugins/core.c, the linux-user caller, and the
> in-tree example plugins.
>
> Signed-off-by: Ziyang Zhang <functioner@sjtu.edu.cn>
> ---
>  include/plugins/qemu-plugin.h | 2 +-
>  include/qemu/plugin.h         | 4 ++--
>  linux-user/syscall.c          | 2 +-
>  plugins/core.c                | 2 +-
>  tests/tcg/plugins/setpc.c     | 2 +-
>  tests/tcg/plugins/syscall.c   | 2 +-
>  6 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
> index 4eb1d2cd85..8eb10b1f48 100644
> --- a/include/plugins/qemu-plugin.h
> +++ b/include/plugins/qemu-plugin.h
> @@ -870,7 +870,7 @@ typedef bool
>                                          int64_t num, uint64_t a1, uint64_t a2,
>                                          uint64_t a3, uint64_t a4, uint64_t a5,
>                                          uint64_t a6, uint64_t a7, uint64_t a8,
> -                                        uint64_t *sysret);
> +                                        int64_t *sysret);
>  
>  /**
>   * typedef qemu_plugin_vcpu_syscall_ret_cb_t - vCPU syscall return callback
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index ddd77bd82c..1ce4b281c1 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -174,7 +174,7 @@ bool
>  qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
>                                  uint64_t a2, uint64_t a3, uint64_t a4,
>                                  uint64_t a5, uint64_t a6, uint64_t a7,
> -                                uint64_t a8, uint64_t *sysret);
> +                                uint64_t a8, int64_t *sysret);
>  
>  void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
>                               uint64_t value_low,
> @@ -290,7 +290,7 @@ static inline bool
>  qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
>                                  uint64_t a2, uint64_t a3, uint64_t a4,
>                                  uint64_t a5, uint64_t a6, uint64_t a7,
> -                                uint64_t a8, uint64_t *sysret)
> +                                uint64_t a8, int64_t *sysret)
>  {
>      return false;
>  }
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f4b74ad350..63c0a5f8f3 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -14378,7 +14378,7 @@ static bool send_through_syscall_filters(CPUState *cpu, int num,
>                                           abi_long arg7, abi_long arg8,
>                                           abi_long *sysret)
>  {
> -    uint64_t sysret64 = 0;
> +    int64_t sysret64 = 0;
>      bool filtered = qemu_plugin_vcpu_syscall_filter(cpu, num, arg1, arg2,
>                                                      arg3, arg4, arg5, arg6,
>                                                      arg7, arg8,
>  &sysret64);

All of the arguments here are abi_long which go to int32_t or
target_long->int64_t so perhaps we should be using that for all args to
ensure signedness is correct?

> diff --git a/plugins/core.c b/plugins/core.c
> index 2324bbffa3..58f293462a 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -596,7 +596,7 @@ bool
>  qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
>                                  uint64_t a2, uint64_t a3, uint64_t a4,
>                                  uint64_t a5, uint64_t a6, uint64_t a7,
> -                                uint64_t a8, uint64_t *sysret)
> +                                uint64_t a8, int64_t *sysret)
>  {
>      struct qemu_plugin_cb *cb, *next;
>      enum qemu_plugin_event ev = QEMU_PLUGIN_EV_VCPU_SYSCALL_FILTER;
> diff --git a/tests/tcg/plugins/setpc.c b/tests/tcg/plugins/setpc.c
> index 8f2d025e24..23862eaaf0 100644
> --- a/tests/tcg/plugins/setpc.c
> +++ b/tests/tcg/plugins/setpc.c
> @@ -27,7 +27,7 @@ static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
>                                  int64_t num, uint64_t a1, uint64_t a2,
>                                  uint64_t a3, uint64_t a4, uint64_t a5,
>                                  uint64_t a6, uint64_t a7, uint64_t a8,
> -                                uint64_t *sysret)
> +                                int64_t *sysret)
>  {
>      if (num == MAGIC_SYSCALL) {
>          if (a1 == SETPC) {
> diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
> index 5658f83087..76d52b98aa 100644
> --- a/tests/tcg/plugins/syscall.c
> +++ b/tests/tcg/plugins/syscall.c
> @@ -174,7 +174,7 @@ static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
>                                  int64_t num, uint64_t a1, uint64_t a2,
>                                  uint64_t a3, uint64_t a4, uint64_t a5,
>                                  uint64_t a6, uint64_t a7, uint64_t a8,
> -                                uint64_t *sysret)
> +                                int64_t *sysret)
>  {
>      /* Special syscall to test the filter functionality. */
>      if (num == 4096 && a1 == 0x66CCFF) {

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] plugins: use int64_t for the syscall filter return value
  2026-06-12 10:43   ` Alex Bennée
@ 2026-06-12 12:48     ` Ziyang Zhang
  0 siblings, 0 replies; 5+ messages in thread
From: Ziyang Zhang @ 2026-06-12 12:48 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Riku Voipio, Laurent Vivier, Alexandre Iooss,
	Mahmoud Mandour, Pierrick Bouvier, Richard Henderson, Zhengwei Qi,
	Yun Wang, Mingyuan Xia, Kailiang Xu

On Fri, 12 Jun 2026 11:43:57 +0100, Alex Bennée wrote:
> All of the arguments here are abi_long which go to int32_t or
> target_long->int64_t so perhaps we should be using that for all args to
> ensure signedness is correct?
You're right that abi_long is signed. I only changed the return value to
keep this callback consistent with the two syscall callbacks that
existed from the first version of the plugin syscall API:
qemu_plugin_vcpu_syscall_cb_t and qemu_plugin_vcpu_syscall_ret_cb_t.
Both use int64_t for num and int64_t for the return value, while the
entry callback passes a1..a8 as uint64_t. This patch matches that.

I'd prefer to keep the arguments as uint64_t. They don't share a single
signedness: pointers and size_t are unsigned, while offsets and fds are
signed. At this boundary they are just register-width words that the
handler casts per syscall, so the correct extension is per-argument-type
(glibc handles them that way too [1]), and forcing them all to int64_t
would be wrong for pointer arguments in particular.

Changing the whole argument vector would also touch several APIs and all
their callers, which is well beyond the scope of this small signedness
fix. So I'd rather keep it to the return value, the one part of this
interface with a well-defined signed meaning.

[1] 
https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h#L50

Thanks,
Ziyang Zhang


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-12 12:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12  5:56 [PATCH 0/1] plugins: fix syscall filter return value type Ziyang Zhang
2026-06-12  5:56 ` [PATCH 1/1] plugins: use int64_t for the syscall filter return value Ziyang Zhang
2026-06-12 10:43   ` Alex Bennée
2026-06-12 12:48     ` Ziyang Zhang
2026-06-12  7:59 ` [PATCH 0/1] plugins: fix syscall filter return value type Ziyang Zhang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.