| Message ID | 20220321131143.31210-1-yuval.shaia.ml@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/pvrdma: Protect against buggy or malicious guest driver | expand |
Hi Yuval On Mon, Mar 21, 2022 at 2:11 PM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote: > > Guest driver might execute HW commands when shared buffers are not yet > allocated. > This might happen on purpose (malicious guest) or because some other > guest/host address mapping. > We need to protect againts such case. > > Reported-by: Mauro Matteo Cascella <mcascell@redhat.com> > Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com> > --- > hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++ > hw/rdma/vmw/pvrdma_main.c | 9 +++++---- > 2 files changed, 11 insertions(+), 4 deletions(-) > > 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"); > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c > index 91206dbb8e..aae382af59 100644 > --- a/hw/rdma/vmw/pvrdma_main.c > +++ b/hw/rdma/vmw/pvrdma_main.c > @@ -159,13 +159,13 @@ static void free_dsr(PVRDMADev *dev) > free_dev_ring(pci_dev, &dev->dsr_info.cq, dev->dsr_info.cq_ring_state); > > rdma_pci_dma_unmap(pci_dev, dev->dsr_info.req, > - sizeof(union pvrdma_cmd_req)); > + sizeof(union pvrdma_cmd_req)); > > rdma_pci_dma_unmap(pci_dev, dev->dsr_info.rsp, > - sizeof(union pvrdma_cmd_resp)); > + sizeof(union pvrdma_cmd_resp)); > > rdma_pci_dma_unmap(pci_dev, dev->dsr_info.dsr, > - sizeof(struct pvrdma_device_shared_region)); > + sizeof(struct pvrdma_device_shared_region)); > Nit: the above changes are not related to the patch, maybe drop them? Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Thanks, Marcel > dev->dsr_info.dsr = NULL; > } > @@ -249,7 +249,8 @@ static void init_dsr_dev_caps(PVRDMADev *dev) > { > struct pvrdma_device_shared_region *dsr; > > - if (dev->dsr_info.dsr == NULL) { > + if (!dev->dsr_info.dsr) { > + /* Buggy or malicious guest driver */ > rdma_error_report("Can't initialized DSR"); > return; > } > -- > 2.20.1 >
On 22/3/22 13:27, Marcel Apfelbaum wrote: > Hi Yuval > > On Mon, Mar 21, 2022 at 2:11 PM Yuval Shaia <yuval.shaia.ml@gmail.com> wrote: >> >> Guest driver might execute HW commands when shared buffers are not yet >> allocated. >> This might happen on purpose (malicious guest) or because some other >> guest/host address mapping. >> We need to protect againts such case. >> Fixes: CVE-2022-1050 >> Reported-by: Mauro Matteo Cascella <mcascell@redhat.com> >> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com> >> --- >> hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++ >> hw/rdma/vmw/pvrdma_main.c | 9 +++++---- >> 2 files changed, 11 insertions(+), 4 deletions(-) >> >> 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"); >> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c >> index 91206dbb8e..aae382af59 100644 >> --- a/hw/rdma/vmw/pvrdma_main.c >> +++ b/hw/rdma/vmw/pvrdma_main.c >> @@ -159,13 +159,13 @@ static void free_dsr(PVRDMADev *dev) >> free_dev_ring(pci_dev, &dev->dsr_info.cq, dev->dsr_info.cq_ring_state); >> >> rdma_pci_dma_unmap(pci_dev, dev->dsr_info.req, >> - sizeof(union pvrdma_cmd_req)); >> + sizeof(union pvrdma_cmd_req)); >> >> rdma_pci_dma_unmap(pci_dev, dev->dsr_info.rsp, >> - sizeof(union pvrdma_cmd_resp)); >> + sizeof(union pvrdma_cmd_resp)); >> >> rdma_pci_dma_unmap(pci_dev, dev->dsr_info.dsr, >> - sizeof(struct pvrdma_device_shared_region)); >> + sizeof(struct pvrdma_device_shared_region)); >> > > Nit: the above changes are not related to the patch, maybe drop them? Yes please. > > Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > Thanks, > Marcel > >> dev->dsr_info.dsr = NULL; >> } >> @@ -249,7 +249,8 @@ static void init_dsr_dev_caps(PVRDMADev *dev) >> { >> struct pvrdma_device_shared_region *dsr; >> >> - if (dev->dsr_info.dsr == NULL) { >> + if (!dev->dsr_info.dsr) { >> + /* Buggy or malicious guest driver */ >> rdma_error_report("Can't initialized DSR"); >> return; >> } >> -- >> 2.20.1 >> >
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"); diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c index 91206dbb8e..aae382af59 100644 --- a/hw/rdma/vmw/pvrdma_main.c +++ b/hw/rdma/vmw/pvrdma_main.c @@ -159,13 +159,13 @@ static void free_dsr(PVRDMADev *dev) free_dev_ring(pci_dev, &dev->dsr_info.cq, dev->dsr_info.cq_ring_state); rdma_pci_dma_unmap(pci_dev, dev->dsr_info.req, - sizeof(union pvrdma_cmd_req)); + sizeof(union pvrdma_cmd_req)); rdma_pci_dma_unmap(pci_dev, dev->dsr_info.rsp, - sizeof(union pvrdma_cmd_resp)); + sizeof(union pvrdma_cmd_resp)); rdma_pci_dma_unmap(pci_dev, dev->dsr_info.dsr, - sizeof(struct pvrdma_device_shared_region)); + sizeof(struct pvrdma_device_shared_region)); dev->dsr_info.dsr = NULL; } @@ -249,7 +249,8 @@ static void init_dsr_dev_caps(PVRDMADev *dev) { struct pvrdma_device_shared_region *dsr; - if (dev->dsr_info.dsr == NULL) { + if (!dev->dsr_info.dsr) { + /* Buggy or malicious guest driver */ rdma_error_report("Can't initialized DSR"); return; }
Guest driver might execute HW commands when shared buffers are not yet allocated. This might happen on purpose (malicious guest) or because some other guest/host address mapping. We need to protect againts such case. Reported-by: Mauro Matteo Cascella <mcascell@redhat.com> Signed-off-by: Yuval Shaia <yuval.shaia.ml@gmail.com> --- hw/rdma/vmw/pvrdma_cmd.c | 6 ++++++ hw/rdma/vmw/pvrdma_main.c | 9 +++++---- 2 files changed, 11 insertions(+), 4 deletions(-)