qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH-for-9.0? v2] backends/cryptodev: Do not abort for invalid session ID
@ 2024-04-09  9:47 Philippe Mathieu-Daudé
  2024-04-09 10:23 ` zhenwei pi
  0 siblings, 1 reply; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-09  9:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Zhenwei Pi, Gonglei (Arei), Alexander Bulekov,
	Philippe Mathieu-Daudé, Zheyu Ma

Instead of aborting when a session ID is invalid,
return VIRTIO_CRYPTO_INVSESS ("Invalid session id").

Reproduced using:

  $ cat << EOF | qemu-system-i386 -display none \
     -machine q35,accel=qtest -m 512M -nodefaults \
     -object cryptodev-backend-builtin,id=cryptodev0 \
     -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
     -qtest stdio
  outl 0xcf8 0x80000804
  outw 0xcfc 0x06
  outl 0xcf8 0x80000820
  outl 0xcfc 0xe0008000
  write 0x10800e 0x1 0x01
  write 0xe0008016 0x1 0x01
  write 0xe0008020 0x4 0x00801000
  write 0xe0008028 0x4 0x00c01000
  write 0xe000801c 0x1 0x01
  write 0x110000 0x1 0x05
  write 0x110001 0x1 0x04
  write 0x108002 0x1 0x11
  write 0x108008 0x1 0x48
  write 0x10800c 0x1 0x01
  write 0x108018 0x1 0x10
  write 0x10801c 0x1 0x02
  write 0x10c002 0x1 0x01
  write 0xe000b005 0x1 0x00
  EOF
  Assertion failed: (session_id < MAX_NUM_SESSIONS && builtin->sessions[session_id]),
  function cryptodev_builtin_close_session, file cryptodev-builtin.c, line 430.

Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2274
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
v2: Removed error_report()
---
 backends/cryptodev-builtin.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 39d0455280..a514bbb310 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -427,7 +427,9 @@ static int cryptodev_builtin_close_session(
                       CRYPTODEV_BACKEND_BUILTIN(backend);
     CryptoDevBackendBuiltinSession *session;
 
-    assert(session_id < MAX_NUM_SESSIONS && builtin->sessions[session_id]);
+    if (session_id >= MAX_NUM_SESSIONS || !builtin->sessions[session_id]) {
+        return -VIRTIO_CRYPTO_INVSESS;
+    }
 
     session = builtin->sessions[session_id];
     if (session->cipher) {
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH-for-9.0? v2] backends/cryptodev: Do not abort for invalid session ID
  2024-04-09  9:47 [PATCH-for-9.0? v2] backends/cryptodev: Do not abort for invalid session ID Philippe Mathieu-Daudé
@ 2024-04-09 10:23 ` zhenwei pi
  2024-04-09 14:36   ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 3+ messages in thread
From: zhenwei pi @ 2024-04-09 10:23 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Gonglei (Arei), Alexander Bulekov, Zheyu Ma

LGTM, thanks!
Reviewed-by: zhenwei pi <pizhenwei@bytedance.com>

On 4/9/24 17:47, Philippe Mathieu-Daudé wrote:
> Instead of aborting when a session ID is invalid,
> return VIRTIO_CRYPTO_INVSESS ("Invalid session id").
> 
> Reproduced using:
> 
>    $ cat << EOF | qemu-system-i386 -display none \
>       -machine q35,accel=qtest -m 512M -nodefaults \
>       -object cryptodev-backend-builtin,id=cryptodev0 \
>       -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
>       -qtest stdio
>    outl 0xcf8 0x80000804
>    outw 0xcfc 0x06
>    outl 0xcf8 0x80000820
>    outl 0xcfc 0xe0008000
>    write 0x10800e 0x1 0x01
>    write 0xe0008016 0x1 0x01
>    write 0xe0008020 0x4 0x00801000
>    write 0xe0008028 0x4 0x00c01000
>    write 0xe000801c 0x1 0x01
>    write 0x110000 0x1 0x05
>    write 0x110001 0x1 0x04
>    write 0x108002 0x1 0x11
>    write 0x108008 0x1 0x48
>    write 0x10800c 0x1 0x01
>    write 0x108018 0x1 0x10
>    write 0x10801c 0x1 0x02
>    write 0x10c002 0x1 0x01
>    write 0xe000b005 0x1 0x00
>    EOF
>    Assertion failed: (session_id < MAX_NUM_SESSIONS && builtin->sessions[session_id]),
>    function cryptodev_builtin_close_session, file cryptodev-builtin.c, line 430.
> 
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2274
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v2: Removed error_report()
> ---
>   backends/cryptodev-builtin.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
> index 39d0455280..a514bbb310 100644
> --- a/backends/cryptodev-builtin.c
> +++ b/backends/cryptodev-builtin.c
> @@ -427,7 +427,9 @@ static int cryptodev_builtin_close_session(
>                         CRYPTODEV_BACKEND_BUILTIN(backend);
>       CryptoDevBackendBuiltinSession *session;
>   
> -    assert(session_id < MAX_NUM_SESSIONS && builtin->sessions[session_id]);
> +    if (session_id >= MAX_NUM_SESSIONS || !builtin->sessions[session_id]) {
> +        return -VIRTIO_CRYPTO_INVSESS;
> +    }
>   
>       session = builtin->sessions[session_id];
>       if (session->cipher) {

-- 
zhenwei pi


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH-for-9.0? v2] backends/cryptodev: Do not abort for invalid session ID
  2024-04-09 10:23 ` zhenwei pi
@ 2024-04-09 14:36   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-09 14:36 UTC (permalink / raw)
  To: zhenwei pi, qemu-devel; +Cc: Gonglei (Arei), Alexander Bulekov, Zheyu Ma

On 9/4/24 12:23, zhenwei pi wrote:
> LGTM, thanks!
> Reviewed-by: zhenwei pi <pizhenwei@bytedance.com>
> 
> On 4/9/24 17:47, Philippe Mathieu-Daudé wrote:
>> Instead of aborting when a session ID is invalid,
>> return VIRTIO_CRYPTO_INVSESS ("Invalid session id").
>>
>> Reproduced using:
>>
>>    $ cat << EOF | qemu-system-i386 -display none \
>>       -machine q35,accel=qtest -m 512M -nodefaults \
>>       -object cryptodev-backend-builtin,id=cryptodev0 \
>>       -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
>>       -qtest stdio
>>    outl 0xcf8 0x80000804
>>    outw 0xcfc 0x06
>>    outl 0xcf8 0x80000820
>>    outl 0xcfc 0xe0008000
>>    write 0x10800e 0x1 0x01
>>    write 0xe0008016 0x1 0x01
>>    write 0xe0008020 0x4 0x00801000
>>    write 0xe0008028 0x4 0x00c01000
>>    write 0xe000801c 0x1 0x01
>>    write 0x110000 0x1 0x05
>>    write 0x110001 0x1 0x04
>>    write 0x108002 0x1 0x11
>>    write 0x108008 0x1 0x48
>>    write 0x10800c 0x1 0x01
>>    write 0x108018 0x1 0x10
>>    write 0x10801c 0x1 0x02
>>    write 0x10c002 0x1 0x01
>>    write 0xe000b005 0x1 0x00
>>    EOF
>>    Assertion failed: (session_id < MAX_NUM_SESSIONS && 
>> builtin->sessions[session_id]),
>>    function cryptodev_builtin_close_session, file cryptodev-builtin.c, 
>> line 430.
>>
>> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2274
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> v2: Removed error_report()
>> ---
>>   backends/cryptodev-builtin.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)

Thanks, patch queued.


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-09 14:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-09  9:47 [PATCH-for-9.0? v2] backends/cryptodev: Do not abort for invalid session ID Philippe Mathieu-Daudé
2024-04-09 10:23 ` zhenwei pi
2024-04-09 14:36   ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).