diff mbox series

[v3] hw/pvrdma: Protect against buggy or malicious guest driver

Message ID 20220403095234.2210-1-yuval.shaia.ml@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v3] hw/pvrdma: Protect against buggy or malicious guest driver | expand

Commit Message

Yuval Shaia April 3, 2022, 9:52 a.m. UTC
Guest driver might execute HW commands when shared buffers are not yet
allocated.
This could happen on purpose (malicious guest) or because of some other
guest/host address mapping error.
We need to protect againts such case.

Fixes: CVE-2022-1050

Reported-by: Raven <wxhusst@gmail.com>
Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
---
v1 -> v2:
	* Commit message changes
v2 -> v3:
	* Exclude cosmetic changes
---
 hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Marcel Apfelbaum April 5, 2022, 10:31 a.m. UTC | #1
Hi Yuval,
Thank you for the changes.

On Sun, Apr 3, 2022 at 11:54 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>
> Guest driver might execute HW commands when shared buffers are not yet
> allocated.
> This could happen on purpose (malicious guest) or because of some other
> guest/host address mapping error.
> We need to protect againts such case.
>
> Fixes: CVE-2022-1050
>
> Reported-by: Raven <wxhusst@gmail.com>
> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
> ---
> v1 -> v2:
>         * Commit message changes
> v2 -> v3:
>         * Exclude cosmetic changes
> ---
>  hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
> index da7ddfa548..89db963c46 100644
> --- a/hw/rdma/vmw/pvrdma_cmd.c
> +++ b/hw/rdma/vmw/pvrdma_cmd.c
> @@ -796,6 +796,12 @@ int pvrdma_exec_cmd(PVRDMADev *dev)
>
>      dsr_info = &dev->dsr_info;
>
> +    if (!dsr_info->dsr) {
> +            /* Buggy or malicious guest driver */
> +            rdma_error_report("Exec command without dsr, req or rsp buffers");
> +            goto out;
> +    }
> +
>      if (dsr_info->req->hdr.cmd >= sizeof(cmd_handlers) /
>                        sizeof(struct cmd_handler)) {
>          rdma_error_report("Unsupported command");
> --
> 2.20.1
>

cc-ing Peter and Philippe for a question:
Do we have a "Security Fixes" or a "Misc" subtree? Otherwise it will
have to wait a week or so.

Reviewed by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Thanks,
Marcel
Michael Tokarev Sept. 12, 2022, 5:45 p.m. UTC | #2
Ping? This is from April this year, half a year ago.
Can this be applied or?

Marcel said it should wait a week or two, I think that's been done already.. ;)

Thanks,

/mjt

03.04.2022 12:52, Yuval Shaia wrote:
> Guest driver might execute HW commands when shared buffers are not yet
> allocated.
> This could happen on purpose (malicious guest) or because of some other
> guest/host address mapping error.
> We need to protect againts such case.
> 
> Fixes: CVE-2022-1050
> 
> Reported-by: Raven <wxhusst@gmail.com>
> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
> ---
> v1 -> v2:
> 	* Commit message changes
> v2 -> v3:
> 	* Exclude cosmetic changes
> ---
>   hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
> index da7ddfa548..89db963c46 100644
> --- a/hw/rdma/vmw/pvrdma_cmd.c
> +++ b/hw/rdma/vmw/pvrdma_cmd.c
> @@ -796,6 +796,12 @@ int pvrdma_exec_cmd(PVRDMADev *dev)
>   
>       dsr_info = &dev->dsr_info;
>   
> +    if (!dsr_info->dsr) {
> +            /* Buggy or malicious guest driver */
> +            rdma_error_report("Exec command without dsr, req or rsp buffers");
> +            goto out;
> +    }
> +
>       if (dsr_info->req->hdr.cmd >= sizeof(cmd_handlers) /
>                         sizeof(struct cmd_handler)) {
>           rdma_error_report("Unsupported command");
Claudio Fontana Dec. 7, 2022, 3:05 p.m. UTC | #3
On 4/5/22 12:31, Marcel Apfelbaum wrote:
> Hi Yuval,
> Thank you for the changes.
> 
> On Sun, Apr 3, 2022 at 11:54 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>>
>> Guest driver might execute HW commands when shared buffers are not yet
>> allocated.
>> This could happen on purpose (malicious guest) or because of some other
>> guest/host address mapping error.
>> We need to protect againts such case.
>>
>> Fixes: CVE-2022-1050
>>
>> Reported-by: Raven <wxhusst@gmail.com>
>> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
>> ---
>> v1 -> v2:
>>         * Commit message changes
>> v2 -> v3:
>>         * Exclude cosmetic changes
>> ---
>>  hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
>> index da7ddfa548..89db963c46 100644
>> --- a/hw/rdma/vmw/pvrdma_cmd.c
>> +++ b/hw/rdma/vmw/pvrdma_cmd.c
>> @@ -796,6 +796,12 @@ int pvrdma_exec_cmd(PVRDMADev *dev)
>>
>>      dsr_info = &dev->dsr_info;
>>
>> +    if (!dsr_info->dsr) {
>> +            /* Buggy or malicious guest driver */
>> +            rdma_error_report("Exec command without dsr, req or rsp buffers");
>> +            goto out;
>> +    }
>> +
>>      if (dsr_info->req->hdr.cmd >= sizeof(cmd_handlers) /
>>                        sizeof(struct cmd_handler)) {
>>          rdma_error_report("Unsupported command");
>> --
>> 2.20.1
>>
> 
> cc-ing Peter and Philippe for a question:
> Do we have a "Security Fixes" or a "Misc" subtree? Otherwise it will
> have to wait a week or so.
> 
> Reviewed by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Thanks,
> Marcel
> 

Hi all,

patch is reviewed, anything holding back the inclusion of this security fix?

Thanks,

Claudio
Yuval Shaia Dec. 19, 2022, 9:57 a.m. UTC | #4
Can anyone else pick this one?

Thanks,
Yuval

On Wed, 7 Dec 2022 at 17:05, Claudio Fontana <cfontana@suse.de> wrote:

> On 4/5/22 12:31, Marcel Apfelbaum wrote:
> > Hi Yuval,
> > Thank you for the changes.
> >
> > On Sun, Apr 3, 2022 at 11:54 AM Yuval Shaia <yuval.shaia.ml@gmail.com>
> wrote:
> >>
> >> Guest driver might execute HW commands when shared buffers are not yet
> >> allocated.
> >> This could happen on purpose (malicious guest) or because of some other
> >> guest/host address mapping error.
> >> We need to protect againts such case.
> >>
> >> Fixes: CVE-2022-1050
> >>
> >> Reported-by: Raven <wxhusst@gmail.com>
> >> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
> >> ---
> >> v1 -> v2:
> >>         * Commit message changes
> >> v2 -> v3:
> >>         * Exclude cosmetic changes
> >> ---
> >>  hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
> >> index da7ddfa548..89db963c46 100644
> >> --- a/hw/rdma/vmw/pvrdma_cmd.c
> >> +++ b/hw/rdma/vmw/pvrdma_cmd.c
> >> @@ -796,6 +796,12 @@ int pvrdma_exec_cmd(PVRDMADev *dev)
> >>
> >>      dsr_info = &dev->dsr_info;
> >>
> >> +    if (!dsr_info->dsr) {
> >> +            /* Buggy or malicious guest driver */
> >> +            rdma_error_report("Exec command without dsr, req or rsp
> buffers");
> >> +            goto out;
> >> +    }
> >> +
> >>      if (dsr_info->req->hdr.cmd >= sizeof(cmd_handlers) /
> >>                        sizeof(struct cmd_handler)) {
> >>          rdma_error_report("Unsupported command");
> >> --
> >> 2.20.1
> >>
> >
> > cc-ing Peter and Philippe for a question:
> > Do we have a "Security Fixes" or a "Misc" subtree? Otherwise it will
> > have to wait a week or so.
> >
> > Reviewed by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > Thanks,
> > Marcel
> >
>
> Hi all,
>
> patch is reviewed, anything holding back the inclusion of this security
> fix?
>
> Thanks,
>
> Claudio
>
Marcel Apfelbaum Dec. 19, 2022, 11:21 a.m. UTC | #5
On Mon, Dec 19, 2022 at 10:57 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>
> Can anyone else pick this one?

Adding Thomas,

I dropped the ball with this one, I am sorry about that, maybe it
doesn't worth a Pull Request only for it.

Maybe it can go through the Misc tree?

Thank you,
Marcel


>
> Thanks,
> Yuval
>
> On Wed, 7 Dec 2022 at 17:05, Claudio Fontana <cfontana@suse.de> wrote:
>>
>> On 4/5/22 12:31, Marcel Apfelbaum wrote:
>> > Hi Yuval,
>> > Thank you for the changes.
>> >
>> > On Sun, Apr 3, 2022 at 11:54 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>> >>
>> >> Guest driver might execute HW commands when shared buffers are not yet
>> >> allocated.
>> >> This could happen on purpose (malicious guest) or because of some other
>> >> guest/host address mapping error.
>> >> We need to protect againts such case.
>> >>
>> >> Fixes: CVE-2022-1050
>> >>
>> >> Reported-by: Raven <wxhusst@gmail.com>
>> >> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
>> >> ---
>> >> v1 -> v2:
>> >>         * Commit message changes
>> >> v2 -> v3:
>> >>         * Exclude cosmetic changes
>> >> ---
>> >>  hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++
>> >>  1 file changed, 6 insertions(+)
>> >>
>> >> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
>> >> index da7ddfa548..89db963c46 100644
>> >> --- a/hw/rdma/vmw/pvrdma_cmd.c
>> >> +++ b/hw/rdma/vmw/pvrdma_cmd.c
>> >> @@ -796,6 +796,12 @@ int pvrdma_exec_cmd(PVRDMADev *dev)
>> >>
>> >>      dsr_info = &dev->dsr_info;
>> >>
>> >> +    if (!dsr_info->dsr) {
>> >> +            /* Buggy or malicious guest driver */
>> >> +            rdma_error_report("Exec command without dsr, req or rsp buffers");
>> >> +            goto out;
>> >> +    }
>> >> +
>> >>      if (dsr_info->req->hdr.cmd >= sizeof(cmd_handlers) /
>> >>                        sizeof(struct cmd_handler)) {
>> >>          rdma_error_report("Unsupported command");
>> >> --
>> >> 2.20.1
>> >>
>> >
>> > cc-ing Peter and Philippe for a question:
>> > Do we have a "Security Fixes" or a "Misc" subtree? Otherwise it will
>> > have to wait a week or so.
>> >
>> > Reviewed by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> > Thanks,
>> > Marcel
>> >
>>
>> Hi all,
>>
>> patch is reviewed, anything holding back the inclusion of this security fix?
>>
>> Thanks,
>>
>> Claudio
Thomas Huth Dec. 28, 2022, 7:32 p.m. UTC | #6
On 19/12/2022 12.21, Marcel Apfelbaum wrote:
> On Mon, Dec 19, 2022 at 10:57 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>>
>> Can anyone else pick this one?
> 
> Adding Thomas,
> 
> I dropped the ball with this one, I am sorry about that, maybe it
> doesn't worth a Pull Request only for it.

Why not? Pull request for single patches aren't that uncommon.

> Maybe it can go through the Misc tree?

hw/rdma/ is really not my turf, but since the patch is small, it sounds like 
a good candidate for qemu-trivial, I think.

  Thomas


>> On Wed, 7 Dec 2022 at 17:05, Claudio Fontana <cfontana@suse.de> wrote:
>>>
>>> On 4/5/22 12:31, Marcel Apfelbaum wrote:
>>>> Hi Yuval,
>>>> Thank you for the changes.
>>>>
>>>> On Sun, Apr 3, 2022 at 11:54 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>>>>>
>>>>> Guest driver might execute HW commands when shared buffers are not yet
>>>>> allocated.
>>>>> This could happen on purpose (malicious guest) or because of some other
>>>>> guest/host address mapping error.
>>>>> We need to protect againts such case.
>>>>>
>>>>> Fixes: CVE-2022-1050
>>>>>
>>>>> Reported-by: Raven <wxhusst@gmail.com>
>>>>> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
>>>>> ---
>>>>> v1 -> v2:
>>>>>          * Commit message changes
>>>>> v2 -> v3:
>>>>>          * Exclude cosmetic changes
>>>>> ---
>>>>>   hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++
>>>>>   1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
>>>>> index da7ddfa548..89db963c46 100644
>>>>> --- a/hw/rdma/vmw/pvrdma_cmd.c
>>>>> +++ b/hw/rdma/vmw/pvrdma_cmd.c
>>>>> @@ -796,6 +796,12 @@ int pvrdma_exec_cmd(PVRDMADev *dev)
>>>>>
>>>>>       dsr_info = &dev->dsr_info;
>>>>>
>>>>> +    if (!dsr_info->dsr) {
>>>>> +            /* Buggy or malicious guest driver */
>>>>> +            rdma_error_report("Exec command without dsr, req or rsp buffers");
>>>>> +            goto out;
>>>>> +    }
>>>>> +
>>>>>       if (dsr_info->req->hdr.cmd >= sizeof(cmd_handlers) /
>>>>>                         sizeof(struct cmd_handler)) {
>>>>>           rdma_error_report("Unsupported command");
>>>>> --
>>>>> 2.20.1
>>>>>
>>>>
>>>> cc-ing Peter and Philippe for a question:
>>>> Do we have a "Security Fixes" or a "Misc" subtree? Otherwise it will
>>>> have to wait a week or so.
>>>>
>>>> Reviewed by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>>> Thanks,
>>>> Marcel
>>>>
>>>
>>> Hi all,
>>>
>>> patch is reviewed, anything holding back the inclusion of this security fix?
>>>
>>> Thanks,
>>>
>>> Claudio
>
Laurent Vivier Jan. 16, 2023, 5:50 p.m. UTC | #7
Le 28/12/2022 à 20:32, Thomas Huth a écrit :
> On 19/12/2022 12.21, Marcel Apfelbaum wrote:
>> On Mon, Dec 19, 2022 at 10:57 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>>>
>>> Can anyone else pick this one?
>>
>> Adding Thomas,
>>
>> I dropped the ball with this one, I am sorry about that, maybe it
>> doesn't worth a Pull Request only for it.
> 
> Why not? Pull request for single patches aren't that uncommon.
> 
>> Maybe it can go through the Misc tree?
> 
> hw/rdma/ is really not my turf, but since the patch is small, it sounds like a good candidate for 
> qemu-trivial, I think.

Applied to my trivial-patches branch.

Thanks,
Laurent

> 
>   Thomas
> 
> 
>>> On Wed, 7 Dec 2022 at 17:05, Claudio Fontana <cfontana@suse.de> wrote:
>>>>
>>>> On 4/5/22 12:31, Marcel Apfelbaum wrote:
>>>>> Hi Yuval,
>>>>> Thank you for the changes.
>>>>>
>>>>> On Sun, Apr 3, 2022 at 11:54 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>>>>>>
>>>>>> Guest driver might execute HW commands when shared buffers are not yet
>>>>>> allocated.
>>>>>> This could happen on purpose (malicious guest) or because of some other
>>>>>> guest/host address mapping error.
>>>>>> We need to protect againts such case.
>>>>>>
>>>>>> Fixes: CVE-2022-1050
>>>>>>
>>>>>> Reported-by: Raven <wxhusst@gmail.com>
>>>>>> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
>>>>>> ---
>>>>>> v1 -> v2:
>>>>>>          * Commit message changes
>>>>>> v2 -> v3:
>>>>>>          * Exclude cosmetic changes
>>>>>> ---
>>>>>>   hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++
>>>>>>   1 file changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
>>>>>> index da7ddfa548..89db963c46 100644
>>>>>> --- a/hw/rdma/vmw/pvrdma_cmd.c
>>>>>> +++ b/hw/rdma/vmw/pvrdma_cmd.c
>>>>>> @@ -796,6 +796,12 @@ int pvrdma_exec_cmd(PVRDMADev *dev)
>>>>>>
>>>>>>       dsr_info = &dev->dsr_info;
>>>>>>
>>>>>> +    if (!dsr_info->dsr) {
>>>>>> +            /* Buggy or malicious guest driver */
>>>>>> +            rdma_error_report("Exec command without dsr, req or rsp buffers");
>>>>>> +            goto out;
>>>>>> +    }
>>>>>> +
>>>>>>       if (dsr_info->req->hdr.cmd >= sizeof(cmd_handlers) /
>>>>>>                         sizeof(struct cmd_handler)) {
>>>>>>           rdma_error_report("Unsupported command");
>>>>>> -- 
>>>>>> 2.20.1
>>>>>>
>>>>>
>>>>> cc-ing Peter and Philippe for a question:
>>>>> Do we have a "Security Fixes" or a "Misc" subtree? Otherwise it will
>>>>> have to wait a week or so.
>>>>>
>>>>> Reviewed by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>>>> Thanks,
>>>>> Marcel
>>>>>
>>>>
>>>> Hi all,
>>>>
>>>> patch is reviewed, anything holding back the inclusion of this security fix?
>>>>
>>>> Thanks,
>>>>
>>>> Claudio
>>
> 
>
Michael Tokarev May 15, 2023, 4:08 p.m. UTC | #8
16.01.2023 20:50, Laurent Vivier wrote:
> Le 28/12/2022 à 20:32, Thomas Huth a écrit :
>> On 19/12/2022 12.21, Marcel Apfelbaum wrote:
>>> On Mon, Dec 19, 2022 at 10:57 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>>>>
>>>> Can anyone else pick this one?
>>>
>>> Adding Thomas,
>>>
>>> I dropped the ball with this one, I am sorry about that, maybe it
>>> doesn't worth a Pull Request only for it.
>>
>> Why not? Pull request for single patches aren't that uncommon.
>>
>>> Maybe it can go through the Misc tree?
>>
>> hw/rdma/ is really not my turf, but since the patch is small, it sounds like a good candidate for qemu-trivial, I think.
> 
> Applied to my trivial-patches branch.

Has it been forgotten again? :)

/mjt
Michael Tokarev May 15, 2023, 4:09 p.m. UTC | #9
15.05.2023 19:08, Michael Tokarev пишет:
> 16.01.2023 20:50, Laurent Vivier wrote:
>> Le 28/12/2022 à 20:32, Thomas Huth a écrit :
>>> On 19/12/2022 12.21, Marcel Apfelbaum wrote:
>>>> On Mon, Dec 19, 2022 at 10:57 AM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote:
>>>>>
>>>>> Can anyone else pick this one?
>>>>
>>>> Adding Thomas,
>>>>
>>>> I dropped the ball with this one, I am sorry about that, maybe it
>>>> doesn't worth a Pull Request only for it.
>>>
>>> Why not? Pull request for single patches aren't that uncommon.
>>>
>>>> Maybe it can go through the Misc tree?
>>>
>>> hw/rdma/ is really not my turf, but since the patch is small, it sounds like a good candidate for qemu-trivial, I think.
>>
>> Applied to my trivial-patches branch.
> 
> Has it been forgotten again? :)

Ah nope. There are 2 patches with the same subject, always confusing.
This one is applied.

/mjt
diff mbox series

Patch

diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index da7ddfa548..89db963c46 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -796,6 +796,12 @@  int pvrdma_exec_cmd(PVRDMADev *dev)
 
     dsr_info = &dev->dsr_info;
 
+    if (!dsr_info->dsr) {
+            /* Buggy or malicious guest driver */
+            rdma_error_report("Exec command without dsr, req or rsp buffers");
+            goto out;
+    }
+
     if (dsr_info->req->hdr.cmd >= sizeof(cmd_handlers) /
                       sizeof(struct cmd_handler)) {
         rdma_error_report("Unsupported command");