Re: [PATCH-for-9.0?] backends/cryptodev: Do not abort for invalid session ID

[zhenwei pi <pizhenwei@bytedance.com>]

Hi,

VIRTIO_CRYPTO_INVSESS has a quite clear meaning: invalid session ID when 
executing crypto operations.

Uplayer would get an explicit code once failing to close session, so I 
suggest no error log printing in this function.

On 4/8/24 23:45, Philippe Mathieu-Daudé wrote:
> Instead of aborting when a session ID is invalid,
> report an error and 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>
> ---
>   backends/cryptodev-builtin.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
> index 39d0455280..3bbaabe86e 100644
> --- a/backends/cryptodev-builtin.c
> +++ b/backends/cryptodev-builtin.c
> @@ -22,6 +22,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>   #include "sysemu/cryptodev.h"
>   #include "qapi/error.h"
>   #include "standard-headers/linux/virtio_crypto.h"
> @@ -427,7 +428,10 @@ 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]) {
> +        error_report("Cannot find a valid session id: %" PRIu64 "", session_id);
> +        return -VIRTIO_CRYPTO_INVSESS;
> +    }
>   
>       session = builtin->sessions[session_id];
>       if (session->cipher) {

-- 
zhenwei pi