* [PATCH 1/4] hw/gpio/pca9552: Avoid using g_newa()
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
@ 2025-06-05 19:35 ` Philippe Mathieu-Daudé
2025-06-05 21:03 ` Miles Glenn
2025-06-05 19:35 ` [PATCH 2/4] backends/tpmL Avoid using g_alloca() Philippe Mathieu-Daudé
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-06-05 19:35 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Philippe Mathieu-Daudé,
Daniel P . Berrangé
We have pin_count <= PCA955X_PIN_COUNT_MAX. Having
PCA955X_PIN_COUNT_MAX = 16, it is safe to explicitly
allocate the char buffer on the stack, without g_newa().
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/gpio/pca9552.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/gpio/pca9552.c b/hw/gpio/pca9552.c
index d65c0a2e90f..1e10238b2e0 100644
--- a/hw/gpio/pca9552.c
+++ b/hw/gpio/pca9552.c
@@ -76,7 +76,7 @@ static void pca955x_display_pins_status(PCA955xState *s,
return;
}
if (trace_event_get_state_backends(TRACE_PCA955X_GPIO_STATUS)) {
- char *buf = g_newa(char, k->pin_count + 1);
+ char buf[PCA955X_PIN_COUNT_MAX + 1];
for (i = 0; i < k->pin_count; i++) {
if (extract32(pins_status, i, 1)) {
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] hw/gpio/pca9552: Avoid using g_newa()
2025-06-05 19:35 ` [PATCH 1/4] hw/gpio/pca9552: Avoid using g_newa() Philippe Mathieu-Daudé
@ 2025-06-05 21:03 ` Miles Glenn
0 siblings, 0 replies; 20+ messages in thread
From: Miles Glenn @ 2025-06-05 21:03 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Marc-André Lureau,
Peter Maydell, Stefan Hajnoczi, Stefan Berger,
Daniel P . Berrangé
Reviewed-by: Glenn Miles <milesg@linux.ibm.com>
Thanks!
Glenn
On Thu, 2025-06-05 at 21:35 +0200, Philippe Mathieu-Daudé wrote:
> We have pin_count <= PCA955X_PIN_COUNT_MAX. Having
> PCA955X_PIN_COUNT_MAX = 16, it is safe to explicitly
> allocate the char buffer on the stack, without g_newa().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/gpio/pca9552.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/gpio/pca9552.c b/hw/gpio/pca9552.c
> index d65c0a2e90f..1e10238b2e0 100644
> --- a/hw/gpio/pca9552.c
> +++ b/hw/gpio/pca9552.c
> @@ -76,7 +76,7 @@ static void pca955x_display_pins_status(PCA955xState *s,
> return;
> }
> if (trace_event_get_state_backends(TRACE_PCA955X_GPIO_STATUS)) {
> - char *buf = g_newa(char, k->pin_count + 1);
> + char buf[PCA955X_PIN_COUNT_MAX + 1];
>
> for (i = 0; i < k->pin_count; i++) {
> if (extract32(pins_status, i, 1)) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/4] backends/tpmL Avoid using g_alloca()
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
2025-06-05 19:35 ` [PATCH 1/4] hw/gpio/pca9552: Avoid using g_newa() Philippe Mathieu-Daudé
@ 2025-06-05 19:35 ` Philippe Mathieu-Daudé
2025-06-05 21:23 ` BALATON Zoltan
` (2 more replies)
2025-06-05 19:35 ` [PATCH 3/4] tests/unit/test-char: " Philippe Mathieu-Daudé
` (6 subsequent siblings)
8 siblings, 3 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-06-05 19:35 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Philippe Mathieu-Daudé,
Daniel P . Berrangé
tpm_emulator_ctrlcmd() is not in hot path.
Use the heap instead of the stack, removing
the g_alloca() call.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
backends/tpm/tpm_emulator.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 43d350e895d..4a234ab2c0b 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -129,11 +129,11 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
CharBackend *dev = &tpm->ctrl_chr;
uint32_t cmd_no = cpu_to_be32(cmd);
ssize_t n = sizeof(uint32_t) + msg_len_in;
- uint8_t *buf = NULL;
ptm_res res;
WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
- buf = g_alloca(n);
+ g_autofree uint8_t *buf = g_malloc(n);
+
memcpy(buf, &cmd_no, sizeof(cmd_no));
memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] backends/tpmL Avoid using g_alloca()
2025-06-05 19:35 ` [PATCH 2/4] backends/tpmL Avoid using g_alloca() Philippe Mathieu-Daudé
@ 2025-06-05 21:23 ` BALATON Zoltan
2025-06-06 7:00 ` Philippe Mathieu-Daudé
2025-06-06 8:14 ` Thomas Huth
2025-06-08 19:07 ` Stefan Berger
2 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2025-06-05 21:23 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]
On Thu, 5 Jun 2025, Philippe Mathieu-Daudé wrote:
> tpm_emulator_ctrlcmd() is not in hot path.
> Use the heap instead of the stack, removing
> the g_alloca() call.
Typo in subject L -> :
Regards,
BALATON Zoltan
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> backends/tpm/tpm_emulator.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 43d350e895d..4a234ab2c0b 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -129,11 +129,11 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
> CharBackend *dev = &tpm->ctrl_chr;
> uint32_t cmd_no = cpu_to_be32(cmd);
> ssize_t n = sizeof(uint32_t) + msg_len_in;
> - uint8_t *buf = NULL;
> ptm_res res;
>
> WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
> - buf = g_alloca(n);
> + g_autofree uint8_t *buf = g_malloc(n);
> +
> memcpy(buf, &cmd_no, sizeof(cmd_no));
> memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] backends/tpmL Avoid using g_alloca()
2025-06-05 21:23 ` BALATON Zoltan
@ 2025-06-06 7:00 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-06-06 7:00 UTC (permalink / raw)
To: BALATON Zoltan
Cc: qemu-devel, qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 5/6/25 23:23, BALATON Zoltan wrote:
> On Thu, 5 Jun 2025, Philippe Mathieu-Daudé wrote:
>> tpm_emulator_ctrlcmd() is not in hot path.
>> Use the heap instead of the stack, removing
>> the g_alloca() call.
>
> Typo in subject L -> :
Oops thanks, I hurt my ring finger and have it now tied with the
middle finger; typing like that is slower and makes me do a lot
of typos ;)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] backends/tpmL Avoid using g_alloca()
2025-06-05 19:35 ` [PATCH 2/4] backends/tpmL Avoid using g_alloca() Philippe Mathieu-Daudé
2025-06-05 21:23 ` BALATON Zoltan
@ 2025-06-06 8:14 ` Thomas Huth
2025-06-08 19:09 ` Stefan Berger
2025-06-08 19:07 ` Stefan Berger
2 siblings, 1 reply; 20+ messages in thread
From: Thomas Huth @ 2025-06-06 8:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 05/06/2025 21.35, Philippe Mathieu-Daudé wrote:
> tpm_emulator_ctrlcmd() is not in hot path.
> Use the heap instead of the stack, removing
> the g_alloca() call.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> backends/tpm/tpm_emulator.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 43d350e895d..4a234ab2c0b 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -129,11 +129,11 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
> CharBackend *dev = &tpm->ctrl_chr;
> uint32_t cmd_no = cpu_to_be32(cmd);
> ssize_t n = sizeof(uint32_t) + msg_len_in;
> - uint8_t *buf = NULL;
> ptm_res res;
>
> WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
> - buf = g_alloca(n);
> + g_autofree uint8_t *buf = g_malloc(n);
> +
> memcpy(buf, &cmd_no, sizeof(cmd_no));
> memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
>
With the typo fixed:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] backends/tpmL Avoid using g_alloca()
2025-06-06 8:14 ` Thomas Huth
@ 2025-06-08 19:09 ` Stefan Berger
0 siblings, 0 replies; 20+ messages in thread
From: Stefan Berger @ 2025-06-08 19:09 UTC (permalink / raw)
To: Thomas Huth, Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 6/6/25 4:14 AM, Thomas Huth wrote:
> On 05/06/2025 21.35, Philippe Mathieu-Daudé wrote:
>> tpm_emulator_ctrlcmd() is not in hot path.
>> Use the heap instead of the stack, removing
>> the g_alloca() call.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> backends/tpm/tpm_emulator.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
>> index 43d350e895d..4a234ab2c0b 100644
>> --- a/backends/tpm/tpm_emulator.c
>> +++ b/backends/tpm/tpm_emulator.c
>> @@ -129,11 +129,11 @@ static int tpm_emulator_ctrlcmd(TPMEmulator
>> *tpm, unsigned long cmd, void *msg,
>> CharBackend *dev = &tpm->ctrl_chr;
>> uint32_t cmd_no = cpu_to_be32(cmd);
>> ssize_t n = sizeof(uint32_t) + msg_len_in;
>> - uint8_t *buf = NULL;
>> ptm_res res;
>> WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
>> - buf = g_alloca(n);
>> + g_autofree uint8_t *buf = g_malloc(n);
>> +
>> memcpy(buf, &cmd_no, sizeof(cmd_no));
>> memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
>
> With the typo fixed:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] backends/tpmL Avoid using g_alloca()
2025-06-05 19:35 ` [PATCH 2/4] backends/tpmL Avoid using g_alloca() Philippe Mathieu-Daudé
2025-06-05 21:23 ` BALATON Zoltan
2025-06-06 8:14 ` Thomas Huth
@ 2025-06-08 19:07 ` Stefan Berger
2 siblings, 0 replies; 20+ messages in thread
From: Stefan Berger @ 2025-06-08 19:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 6/5/25 3:35 PM, Philippe Mathieu-Daudé wrote:
> tpm_emulator_ctrlcmd() is not in hot path.
> Use the heap instead of the stack, removing
> the g_alloca() call.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> backends/tpm/tpm_emulator.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 43d350e895d..4a234ab2c0b 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -129,11 +129,11 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
> CharBackend *dev = &tpm->ctrl_chr;
> uint32_t cmd_no = cpu_to_be32(cmd);
> ssize_t n = sizeof(uint32_t) + msg_len_in;
> - uint8_t *buf = NULL;
> ptm_res res;
>
> WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
> - buf = g_alloca(n);
> + g_autofree uint8_t *buf = g_malloc(n);
> +
> memcpy(buf, &cmd_no, sizeof(cmd_no));
> memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/4] tests/unit/test-char: Avoid using g_alloca()
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
2025-06-05 19:35 ` [PATCH 1/4] hw/gpio/pca9552: Avoid using g_newa() Philippe Mathieu-Daudé
2025-06-05 19:35 ` [PATCH 2/4] backends/tpmL Avoid using g_alloca() Philippe Mathieu-Daudé
@ 2025-06-05 19:35 ` Philippe Mathieu-Daudé
2025-06-05 20:53 ` Pierrick Bouvier
2025-06-05 19:35 ` [RFC PATCH 4/4] buildsys: Prohibit alloca() use on system code Philippe Mathieu-Daudé
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-06-05 19:35 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Philippe Mathieu-Daudé,
Daniel P . Berrangé
Do not use g_alloca(), simply allocate the CharBackend
structure on the stack.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/unit/test-char.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index 60a843b79d9..f30a39f61ff 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -993,7 +993,7 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock)
struct sockaddr_in other;
SocketIdleData d = { 0, };
Chardev *chr;
- CharBackend *be;
+ CharBackend stack_be, *be = &stack_be;
socklen_t alen = sizeof(other);
int ret;
char buf[10];
@@ -1009,7 +1009,6 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock)
chr = qemu_chr_new("client", tmp, NULL);
g_assert_nonnull(chr);
- be = g_alloca(sizeof(CharBackend));
qemu_chr_fe_init(be, chr, &error_abort);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] tests/unit/test-char: Avoid using g_alloca()
2025-06-05 19:35 ` [PATCH 3/4] tests/unit/test-char: " Philippe Mathieu-Daudé
@ 2025-06-05 20:53 ` Pierrick Bouvier
2025-06-06 6:09 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 20+ messages in thread
From: Pierrick Bouvier @ 2025-06-05 20:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 6/5/25 12:35 PM, Philippe Mathieu-Daudé wrote:
> Do not use g_alloca(), simply allocate the CharBackend
> structure on the stack.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> tests/unit/test-char.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
> index 60a843b79d9..f30a39f61ff 100644
> --- a/tests/unit/test-char.c
> +++ b/tests/unit/test-char.c
> @@ -993,7 +993,7 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock)
> struct sockaddr_in other;
> SocketIdleData d = { 0, };
> Chardev *chr;
> - CharBackend *be;
> + CharBackend stack_be, *be = &stack_be;
> socklen_t alen = sizeof(other);
> int ret;
> char buf[10];
> @@ -1009,7 +1009,6 @@ static void char_udp_test_internal(Chardev *reuse_chr, int sock)
> chr = qemu_chr_new("client", tmp, NULL);
> g_assert_nonnull(chr);
>
> - be = g_alloca(sizeof(CharBackend));
> qemu_chr_fe_init(be, chr, &error_abort);
> }
>
Would that be more simple to declare the variable, and use &be in the
function code?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] tests/unit/test-char: Avoid using g_alloca()
2025-06-05 20:53 ` Pierrick Bouvier
@ 2025-06-06 6:09 ` Philippe Mathieu-Daudé
2025-06-06 16:18 ` Pierrick Bouvier
0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-06-06 6:09 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 5/6/25 22:53, Pierrick Bouvier wrote:
> On 6/5/25 12:35 PM, Philippe Mathieu-Daudé wrote:
>> Do not use g_alloca(), simply allocate the CharBackend
>> structure on the stack.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> tests/unit/test-char.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
>> index 60a843b79d9..f30a39f61ff 100644
>> --- a/tests/unit/test-char.c
>> +++ b/tests/unit/test-char.c
>> @@ -993,7 +993,7 @@ static void char_udp_test_internal(Chardev
>> *reuse_chr, int sock)
>> struct sockaddr_in other;
>> SocketIdleData d = { 0, };
>> Chardev *chr;
>> - CharBackend *be;
>> + CharBackend stack_be, *be = &stack_be;
>> socklen_t alen = sizeof(other);
>> int ret;
>> char buf[10];
>> @@ -1009,7 +1009,6 @@ static void char_udp_test_internal(Chardev
>> *reuse_chr, int sock)
>> chr = qemu_chr_new("client", tmp, NULL);
>> g_assert_nonnull(chr);
>> - be = g_alloca(sizeof(CharBackend));
>> qemu_chr_fe_init(be, chr, &error_abort);
>> }
>
> Would that be more simple to declare the variable, and use &be in the
> function code?
More context (see reuse_chr ladder):
-- >8 --
@@ -991,45 +991,44 @@ static int make_udp_socket(int *port)
static void char_udp_test_internal(Chardev *reuse_chr, int sock)
{
struct sockaddr_in other;
SocketIdleData d = { 0, };
Chardev *chr;
- CharBackend *be;
+ CharBackend stack_be, *be = &stack_be;
socklen_t alen = sizeof(other);
int ret;
char buf[10];
char *tmp = NULL;
if (reuse_chr) {
chr = reuse_chr;
be = chr->be;
} else {
int port;
sock = make_udp_socket(&port);
tmp = g_strdup_printf("udp:127.0.0.1:%d", port);
chr = qemu_chr_new("client", tmp, NULL);
g_assert_nonnull(chr);
- be = g_alloca(sizeof(CharBackend));
qemu_chr_fe_init(be, chr, &error_abort);
}
---
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] tests/unit/test-char: Avoid using g_alloca()
2025-06-06 6:09 ` Philippe Mathieu-Daudé
@ 2025-06-06 16:18 ` Pierrick Bouvier
0 siblings, 0 replies; 20+ messages in thread
From: Pierrick Bouvier @ 2025-06-06 16:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 6/5/25 11:09 PM, Philippe Mathieu-Daudé wrote:
> On 5/6/25 22:53, Pierrick Bouvier wrote:
>> On 6/5/25 12:35 PM, Philippe Mathieu-Daudé wrote:
>>> Do not use g_alloca(), simply allocate the CharBackend
>>> structure on the stack.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> tests/unit/test-char.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
>>> index 60a843b79d9..f30a39f61ff 100644
>>> --- a/tests/unit/test-char.c
>>> +++ b/tests/unit/test-char.c
>>> @@ -993,7 +993,7 @@ static void char_udp_test_internal(Chardev
>>> *reuse_chr, int sock)
>>> struct sockaddr_in other;
>>> SocketIdleData d = { 0, };
>>> Chardev *chr;
>>> - CharBackend *be;
>>> + CharBackend stack_be, *be = &stack_be;
>>> socklen_t alen = sizeof(other);
>>> int ret;
>>> char buf[10];
>>> @@ -1009,7 +1009,6 @@ static void char_udp_test_internal(Chardev
>>> *reuse_chr, int sock)
>>> chr = qemu_chr_new("client", tmp, NULL);
>>> g_assert_nonnull(chr);
>>> - be = g_alloca(sizeof(CharBackend));
>>> qemu_chr_fe_init(be, chr, &error_abort);
>>> }
>>
>> Would that be more simple to declare the variable, and use &be in the
>> function code?
>
> More context (see reuse_chr ladder):
>
> -- >8 --
> @@ -991,45 +991,44 @@ static int make_udp_socket(int *port)
> static void char_udp_test_internal(Chardev *reuse_chr, int sock)
> {
> struct sockaddr_in other;
> SocketIdleData d = { 0, };
> Chardev *chr;
> - CharBackend *be;
> + CharBackend stack_be, *be = &stack_be;
> socklen_t alen = sizeof(other);
> int ret;
> char buf[10];
> char *tmp = NULL;
>
> if (reuse_chr) {
> chr = reuse_chr;
> be = chr->be;
> } else {
> int port;
> sock = make_udp_socket(&port);
> tmp = g_strdup_printf("udp:127.0.0.1:%d", port);
> chr = qemu_chr_new("client", tmp, NULL);
> g_assert_nonnull(chr);
>
> - be = g_alloca(sizeof(CharBackend));
> qemu_chr_fe_init(be, chr, &error_abort);
> }
> ---
Ok!
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC PATCH 4/4] buildsys: Prohibit alloca() use on system code
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2025-06-05 19:35 ` [PATCH 3/4] tests/unit/test-char: " Philippe Mathieu-Daudé
@ 2025-06-05 19:35 ` Philippe Mathieu-Daudé
2025-06-05 20:53 ` [PATCH 0/4] system: Forbid alloca() Pierrick Bouvier
` (4 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-06-05 19:35 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Philippe Mathieu-Daudé,
Daniel P . Berrangé
Similarly to commit 64c1a544352 ("meson: Enable -Wvla") with
variable length arrays, forbid alloca() uses on system code.
There are few uses on ancient linux-user code, do not bother
there.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
meson.build | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/meson.build b/meson.build
index ef994676fe9..8c6ccb03c71 100644
--- a/meson.build
+++ b/meson.build
@@ -774,6 +774,10 @@ if host_os != 'darwin'
endif
endif
+if have_system
+ warn_flags += ['-Walloca']
+endif
+
# Set up C++ compiler flags
qemu_cxxflags = []
if 'cpp' in all_languages
--
2.49.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] system: Forbid alloca()
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2025-06-05 19:35 ` [RFC PATCH 4/4] buildsys: Prohibit alloca() use on system code Philippe Mathieu-Daudé
@ 2025-06-05 20:53 ` Pierrick Bouvier
2025-06-06 8:37 ` Peter Maydell
` (3 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Pierrick Bouvier @ 2025-06-05 20:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 6/5/25 12:35 PM, Philippe Mathieu-Daudé wrote:
> Eradicate alloca() uses on system code, then enable
> -Walloca to prevent new ones to creep back in.
>
> Philippe Mathieu-Daudé (4):
> hw/gpio/pca9552: Avoid using g_newa()
> backends/tpmL Avoid using g_alloca()
> tests/unit/test-char: Avoid using g_alloca()
> buildsys: Prohibit alloca() use on system code
>
> meson.build | 4 ++++
> backends/tpm/tpm_emulator.c | 4 ++--
> hw/gpio/pca9552.c | 2 +-
> tests/unit/test-char.c | 3 +--
> 4 files changed, 8 insertions(+), 5 deletions(-)
>
Good idea!
For the series:
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] system: Forbid alloca()
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2025-06-05 20:53 ` [PATCH 0/4] system: Forbid alloca() Pierrick Bouvier
@ 2025-06-06 8:37 ` Peter Maydell
2025-06-06 8:53 ` Philippe Mathieu-Daudé
2025-06-06 8:44 ` Alex Bennée
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Peter Maydell @ 2025-06-06 8:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Stefan Hajnoczi, Stefan Berger,
Daniel P . Berrangé
On Thu, 5 Jun 2025 at 20:35, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Eradicate alloca() uses on system code, then enable
> -Walloca to prevent new ones to creep back in.
>
> Philippe Mathieu-Daudé (4):
> hw/gpio/pca9552: Avoid using g_newa()
> backends/tpmL Avoid using g_alloca()
> tests/unit/test-char: Avoid using g_alloca()
> buildsys: Prohibit alloca() use on system code
>
> meson.build | 4 ++++
> backends/tpm/tpm_emulator.c | 4 ++--
> hw/gpio/pca9552.c | 2 +-
> tests/unit/test-char.c | 3 +--
> 4 files changed, 8 insertions(+), 5 deletions(-)
There is also a use of alloca() in target/ppc/kvm.c
in kvmppc_load_htab_chunk(), so I suspect that patch 4
here will break compilation on PPC hosts with KVM enabled.
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] system: Forbid alloca()
2025-06-06 8:37 ` Peter Maydell
@ 2025-06-06 8:53 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-06-06 8:53 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Stefan Hajnoczi, Stefan Berger,
Daniel P . Berrangé
On 6/6/25 10:37, Peter Maydell wrote:
> On Thu, 5 Jun 2025 at 20:35, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Eradicate alloca() uses on system code, then enable
>> -Walloca to prevent new ones to creep back in.
>>
>> Philippe Mathieu-Daudé (4):
>> hw/gpio/pca9552: Avoid using g_newa()
>> backends/tpmL Avoid using g_alloca()
>> tests/unit/test-char: Avoid using g_alloca()
>> buildsys: Prohibit alloca() use on system code
>>
>> meson.build | 4 ++++
>> backends/tpm/tpm_emulator.c | 4 ++--
>> hw/gpio/pca9552.c | 2 +-
>> tests/unit/test-char.c | 3 +--
>> 4 files changed, 8 insertions(+), 5 deletions(-)
>
> There is also a use of alloca() in target/ppc/kvm.c
> in kvmppc_load_htab_chunk(), so I suspect that patch 4
> here will break compilation on PPC hosts with KVM enabled.
Oops sorry I missed that one :/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] system: Forbid alloca()
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2025-06-06 8:37 ` Peter Maydell
@ 2025-06-06 8:44 ` Alex Bennée
2025-06-09 13:46 ` Stefan Hajnoczi
2025-06-10 9:29 ` Philippe Mathieu-Daudé
8 siblings, 0 replies; 20+ messages in thread
From: Alex Bennée @ 2025-06-06 8:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> Eradicate alloca() uses on system code, then enable
> -Walloca to prevent new ones to creep back in.
Should we also mention it in style.rst:
Use of the ``malloc/free/realloc/calloc/valloc/memalign/posix_memalign``
APIs is not allowed in the QEMU codebase. Instead of these routines,
>
> Philippe Mathieu-Daudé (4):
> hw/gpio/pca9552: Avoid using g_newa()
> backends/tpmL Avoid using g_alloca()
> tests/unit/test-char: Avoid using g_alloca()
> buildsys: Prohibit alloca() use on system code
>
> meson.build | 4 ++++
> backends/tpm/tpm_emulator.c | 4 ++--
> hw/gpio/pca9552.c | 2 +-
> tests/unit/test-char.c | 3 +--
> 4 files changed, 8 insertions(+), 5 deletions(-)
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] system: Forbid alloca()
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2025-06-06 8:44 ` Alex Bennée
@ 2025-06-09 13:46 ` Stefan Hajnoczi
2025-06-10 9:29 ` Philippe Mathieu-Daudé
8 siblings, 0 replies; 20+ messages in thread
From: Stefan Hajnoczi @ 2025-06-09 13:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Berger,
Daniel P . Berrangé
[-- Attachment #1: Type: text/plain, Size: 729 bytes --]
On Thu, Jun 05, 2025 at 09:35:36PM +0200, Philippe Mathieu-Daudé wrote:
> Eradicate alloca() uses on system code, then enable
> -Walloca to prevent new ones to creep back in.
>
> Philippe Mathieu-Daudé (4):
> hw/gpio/pca9552: Avoid using g_newa()
> backends/tpmL Avoid using g_alloca()
> tests/unit/test-char: Avoid using g_alloca()
> buildsys: Prohibit alloca() use on system code
>
> meson.build | 4 ++++
> backends/tpm/tpm_emulator.c | 4 ++--
> hw/gpio/pca9552.c | 2 +-
> tests/unit/test-char.c | 3 +--
> 4 files changed, 8 insertions(+), 5 deletions(-)
Modulo the comments that have already been discussed:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] system: Forbid alloca()
2025-06-05 19:35 [PATCH 0/4] system: Forbid alloca() Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2025-06-09 13:46 ` Stefan Hajnoczi
@ 2025-06-10 9:29 ` Philippe Mathieu-Daudé
8 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-06-10 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, Paolo Bonzini, qemu-arm, Glenn Miles,
Marc-André Lureau, Peter Maydell, Stefan Hajnoczi,
Stefan Berger, Daniel P . Berrangé
On 5/6/25 21:35, Philippe Mathieu-Daudé wrote:
> Eradicate alloca() uses on system code, then enable
> -Walloca to prevent new ones to creep back in.
>
> Philippe Mathieu-Daudé (4):
> hw/gpio/pca9552: Avoid using g_newa()
> backends/tpmL Avoid using g_alloca()
> tests/unit/test-char: Avoid using g_alloca()
Patches 1-3 queued.
^ permalink raw reply [flat|nested] 20+ messages in thread