diff mbox series

[PULL,22/22] cryptodev: Handle unexpected request to avoid crash

Message ID 15b11a1da6a4b7c6b8bb37883f52b544dee2b8fd.1691101215.git.mst@redhat.com
State New
Headers show
Series [PULL,01/22] hw/virtio-iommu: Fix potential OOB access in virtio_iommu_handle_command() | expand

Commit Message

Michael S. Tsirkin Aug. 3, 2023, 10:21 p.m. UTC
From: zhenwei pi <pizhenwei@bytedance.com>

Generally guest side should discover which services the device is
able to offer, then do requests on device.

However it's also possible to break this rule in a guest. Handle
unexpected request here to avoid NULL pointer dereference.

Fixes: e7a775fd ('cryptodev: Account statistics')
Cc: Gonglei <arei.gonglei@huawei.com>
Cc: Mauro Matteo Cascella <mcascell@redhat.com>
Cc: Xiao Lei <nop.leixiao@gmail.com>
Cc: Yongkang Jia <kangel@zju.edu.cn>
Reported-by: Yiming Tao <taoym@zju.edu.cn>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230803024314.29962-3-pizhenwei@bytedance.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 backends/cryptodev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Michael Tokarev Aug. 4, 2023, 4:35 a.m. UTC | #1
04.08.2023 01:21, Michael S. Tsirkin wrote:
> From: zhenwei pi <pizhenwei@bytedance.com>
> 
> Generally guest side should discover which services the device is
> able to offer, then do requests on device.
> 
> However it's also possible to break this rule in a guest. Handle
> unexpected request here to avoid NULL pointer dereference.
> 
> Fixes: e7a775fd ('cryptodev: Account statistics')
> Cc: Gonglei <arei.gonglei@huawei.com>
> Cc: Mauro Matteo Cascella <mcascell@redhat.com>
> Cc: Xiao Lei <nop.leixiao@gmail.com>
> Cc: Yongkang Jia <kangel@zju.edu.cn>
> Reported-by: Yiming Tao <taoym@zju.edu.cn>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> Message-Id: <20230803024314.29962-3-pizhenwei@bytedance.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

This smells like a stable-8.0 material. Please let me know if it is not.

Thanks,

/mjt
zhenwei pi Aug. 4, 2023, 6:10 a.m. UTC | #2
On 8/4/23 12:35, Michael Tokarev wrote:
> 04.08.2023 01:21, Michael S. Tsirkin wrote:
>> From: zhenwei pi <pizhenwei@bytedance.com>
>>
>> Generally guest side should discover which services the device is
>> able to offer, then do requests on device.
>>
>> However it's also possible to break this rule in a guest. Handle
>> unexpected request here to avoid NULL pointer dereference.
>>
>> Fixes: e7a775fd ('cryptodev: Account statistics')
>> Cc: Gonglei <arei.gonglei@huawei.com>
>> Cc: Mauro Matteo Cascella <mcascell@redhat.com>
>> Cc: Xiao Lei <nop.leixiao@gmail.com>
>> Cc: Yongkang Jia <kangel@zju.edu.cn>
>> Reported-by: Yiming Tao <taoym@zju.edu.cn>
>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>> Message-Id: <20230803024314.29962-3-pizhenwei@bytedance.com>
>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> This smells like a stable-8.0 material. Please let me know if it is not.
> 
> Thanks,
> 
> /mjt

Hi,

Both this one [1] and the previous one [2] should be table-8.0 material, 
I think.

[1][PULL 22/22] cryptodev: Handle unexpected request to avoid crash
[2][PULL 21/22] virtio-crypto: verify src&dst buffer length for sym request
Michael Tokarev Aug. 4, 2023, 6:35 a.m. UTC | #3
04.08.2023 09:10, zhenwei pi wrote:
..
>> This smells like a stable-8.0 material. Please let me know if it is not.

> Both this one [1] and the previous one [2] should be table-8.0 material, I think.
> 
> [1][PULL 22/22] cryptodev: Handle unexpected request to avoid crash
> [2][PULL 21/22] virtio-crypto: verify src&dst buffer length for sym request

I picked up the 21 one (virtio-crypto) already without questions (for both 8.0 and 7.2)
since it fixes a CVE, but I wasn't 100% sure for the 22 one (cryptodev).

Thank you for letting me know.

/mjt
diff mbox series

Patch

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 7d29517843..4d183f7237 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -191,6 +191,11 @@  static int cryptodev_backend_account(CryptoDevBackend *backend,
     if (algtype == QCRYPTODEV_BACKEND_ALG_ASYM) {
         CryptoDevBackendAsymOpInfo *asym_op_info = op_info->u.asym_op_info;
         len = asym_op_info->src_len;
+
+        if (unlikely(!backend->asym_stat)) {
+            error_report("cryptodev: Unexpected asym operation");
+            return -VIRTIO_CRYPTO_NOTSUPP;
+        }
         switch (op_info->op_code) {
         case VIRTIO_CRYPTO_AKCIPHER_ENCRYPT:
             CryptodevAsymStatIncEncrypt(backend, len);
@@ -210,6 +215,11 @@  static int cryptodev_backend_account(CryptoDevBackend *backend,
     } else if (algtype == QCRYPTODEV_BACKEND_ALG_SYM) {
         CryptoDevBackendSymOpInfo *sym_op_info = op_info->u.sym_op_info;
         len = sym_op_info->src_len;
+
+        if (unlikely(!backend->sym_stat)) {
+            error_report("cryptodev: Unexpected sym operation");
+            return -VIRTIO_CRYPTO_NOTSUPP;
+        }
         switch (op_info->op_code) {
         case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
             CryptodevSymStatIncEncrypt(backend, len);