[PATCH] vhost/crypto: fix segfault

[PATCH] vhost/crypto: fix segfault

[Radu Nicolau]

Fix potential call with dev->mem uninitialized, one common usecase
example being running the autotest with more than one device.

Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")
Cc: stable@dpdk.org

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 lib/vhost/vhost_crypto.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
index 648e2d731b..3679eaca1e 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -1512,6 +1512,10 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 		VC_LOG_ERR("Invalid descriptor");
 		return -1;
 	}
+	if (unlikely((vc_req->dev->mem) == NULL)) {
+		VC_LOG_ERR("Uninitialized vhost device");
+		return -1;
+	}
 
 	dlen = head->len;
 	src_desc = IOVA_TO_VVA(struct vring_desc *, vc_req->dev, vq,
-- 
2.52.0

Re: [PATCH] vhost/crypto: fix segfault

[David Marchand]

On Wed, 24 Jun 2026 at 16:21, Radu Nicolau <radu.nicolau@intel.com> wrote:
>
> Fix potential call with dev->mem uninitialized, one common usecase
> example being running the autotest with more than one device.
>
> Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")
> Cc: stable@dpdk.org
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
>  lib/vhost/vhost_crypto.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
> index 648e2d731b..3679eaca1e 100644
> --- a/lib/vhost/vhost_crypto.c
> +++ b/lib/vhost/vhost_crypto.c
> @@ -1512,6 +1512,10 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
>                 VC_LOG_ERR("Invalid descriptor");
>                 return -1;
>         }
> +       if (unlikely((vc_req->dev->mem) == NULL)) {

(Unneeded extra ())

It sounds to me that some initialisation failed, or processing happens
without waiting initialisation finished.


> +               VC_LOG_ERR("Uninitialized vhost device");
> +               return -1;
> +       }
>
>         dlen = head->len;
>         src_desc = IOVA_TO_VVA(struct vring_desc *, vc_req->dev, vq,
> --
> 2.52.0
>


-- 
David Marchand

Re: [PATCH] vhost/crypto: fix segfault

[Radu Nicolau]

On 25-Jun-26 1:44 PM, David Marchand wrote:
> On Wed, 24 Jun 2026 at 16:21, Radu Nicolau <radu.nicolau@intel.com> wrote:
>> Fix potential call with dev->mem uninitialized, one common usecase
>> example being running the autotest with more than one device.
>>
>> Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---
>>   lib/vhost/vhost_crypto.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
>> index 648e2d731b..3679eaca1e 100644
>> --- a/lib/vhost/vhost_crypto.c
>> +++ b/lib/vhost/vhost_crypto.c
>> @@ -1512,6 +1512,10 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
>>                  VC_LOG_ERR("Invalid descriptor");
>>                  return -1;
>>          }
>> +       if (unlikely((vc_req->dev->mem) == NULL)) {
> (Unneeded extra ())
>
> It sounds to me that some initialisation failed, or processing happens
> without waiting initialisation finished.

Indeed calling this function with dev->mem uninitialized is caused by 
some init/deinit state bug, but I still think we should keep this check 
in for added safety.

I will follow up with a v2 to remove the redundant () and also I will 
look into fixing the root cause.

>
>
>> +               VC_LOG_ERR("Uninitialized vhost device");
>> +               return -1;
>> +       }
>>
>>          dlen = head->len;
>>          src_desc = IOVA_TO_VVA(struct vring_desc *, vc_req->dev, vq,
>> --
>> 2.52.0
>>
>

[PATCH v2] vhost/crypto: fix segfault

[Radu Nicolau]

Fix potential call with dev->mem uninitialized, one common usecase
example being running the autotest with more than one device.

Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")
Cc: stable@dpdk.org

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
v2: removed unncessary ()

 lib/vhost/vhost_crypto.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
index 648e2d731b..a19cdd61cc 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -1512,6 +1512,10 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 		VC_LOG_ERR("Invalid descriptor");
 		return -1;
 	}
+	if (unlikely(vc_req->dev->mem == NULL)) {
+		VC_LOG_ERR("Uninitialized vhost device");
+		return -1;
+	}
 
 	dlen = head->len;
 	src_desc = IOVA_TO_VVA(struct vring_desc *, vc_req->dev, vq,
-- 
2.52.0

Re: [PATCH] vhost/crypto: fix segfault

[Maxime Coquelin]

On Fri, Jun 26, 2026 at 10:50 AM Radu Nicolau <radu.nicolau@intel.com> wrote:
>
>
> On 25-Jun-26 1:44 PM, David Marchand wrote:
> > On Wed, 24 Jun 2026 at 16:21, Radu Nicolau <radu.nicolau@intel.com> wrote:
> >> Fix potential call with dev->mem uninitialized, one common usecase
> >> example being running the autotest with more than one device.
> >>
> >> Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> >> ---
> >>   lib/vhost/vhost_crypto.c | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
> >> index 648e2d731b..3679eaca1e 100644
> >> --- a/lib/vhost/vhost_crypto.c
> >> +++ b/lib/vhost/vhost_crypto.c
> >> @@ -1512,6 +1512,10 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
> >>                  VC_LOG_ERR("Invalid descriptor");
> >>                  return -1;
> >>          }
> >> +       if (unlikely((vc_req->dev->mem) == NULL)) {
> > (Unneeded extra ())
> >
> > It sounds to me that some initialisation failed, or processing happens
> > without waiting initialisation finished.
>
> Indeed calling this function with dev->mem uninitialized is caused by
> some init/deinit state bug, but I still think we should keep this check
> in for added safety.
>
> I will follow up with a v2 to remove the redundant () and also I will
> look into fixing the root cause.

Or maybe this crypto backend is lacking proper protection between
control and data paths?
If dev->mem is reset to NULL asynchrously by the control path without
proper locking, then
the above fix is not enough. For example, the above check could pass
and dev->mem be
reset right after.

>
> >
> >
> >> +               VC_LOG_ERR("Uninitialized vhost device");
> >> +               return -1;
> >> +       }
> >>
> >>          dlen = head->len;
> >>          src_desc = IOVA_TO_VVA(struct vring_desc *, vc_req->dev, vq,
> >> --
> >> 2.52.0
> >>
> >
>

Re: [PATCH] vhost/crypto: fix segfault

[Radu Nicolau]

On 29-Jun-26 9:18 AM, Maxime Coquelin wrote:
> On Fri, Jun 26, 2026 at 10:50 AM Radu Nicolau <radu.nicolau@intel.com> wrote:
>>
>> On 25-Jun-26 1:44 PM, David Marchand wrote:
>>> On Wed, 24 Jun 2026 at 16:21, Radu Nicolau <radu.nicolau@intel.com> wrote:
>>>> Fix potential call with dev->mem uninitialized, one common usecase
>>>> example being running the autotest with more than one device.
>>>>
>>>> Fixes: 3bb595ecd682 ("vhost/crypto: add request handler")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>>>> ---
>>>>    lib/vhost/vhost_crypto.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
>>>> index 648e2d731b..3679eaca1e 100644
>>>> --- a/lib/vhost/vhost_crypto.c
>>>> +++ b/lib/vhost/vhost_crypto.c
>>>> @@ -1512,6 +1512,10 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
>>>>                   VC_LOG_ERR("Invalid descriptor");
>>>>                   return -1;
>>>>           }
>>>> +       if (unlikely((vc_req->dev->mem) == NULL)) {
>>> (Unneeded extra ())
>>>
>>> It sounds to me that some initialisation failed, or processing happens
>>> without waiting initialisation finished.
>> Indeed calling this function with dev->mem uninitialized is caused by
>> some init/deinit state bug, but I still think we should keep this check
>> in for added safety.
>>
>> I will follow up with a v2 to remove the redundant () and also I will
>> look into fixing the root cause.
> Or maybe this crypto backend is lacking proper protection between
> control and data paths?
> If dev->mem is reset to NULL asynchrously by the control path without
> proper locking, then
> the above fix is not enough. For example, the above check could pass
> and dev->mem be
> reset right after.
100% there is a deeper root cause for this segfault and I'm working to 
find it, but it may take a while. That's why I propose this as a stop 
gap fix, it adds an extra check but of course it doesn't remove the root 
cause.