lkml.org 
[lkml]   [2022]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v2 1/2] PCI: hv: Add validation for untrusted Hyper-V values
    Date
    From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, May 11, 2022 3:32 PM
    >
    > For additional robustness in the face of Hyper-V errors or malicious
    > behavior, validate all values that originate from packets that Hyper-V
    > has sent to the guest in the host-to-guest ring buffer. Ensure that
    > invalid values cannot cause data being copied out of the bounds of the
    > source buffer in hv_pci_onchannelcallback().
    >
    > While at it, remove a redundant validation in hv_pci_generic_compl():
    > hv_pci_onchannelcallback() already ensures that all processed incoming
    > packets are "at least as large as [in fact larger than] a response".
    >
    > Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
    > ---
    > drivers/pci/controller/pci-hyperv.c | 33 +++++++++++++++++++++--------
    > 1 file changed, 24 insertions(+), 9 deletions(-)
    >
    > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
    > index e439b810f974b..a06e2cf946580 100644
    > --- a/drivers/pci/controller/pci-hyperv.c
    > +++ b/drivers/pci/controller/pci-hyperv.c
    > @@ -981,11 +981,7 @@ static void hv_pci_generic_compl(void *context, struct
    > pci_response *resp,
    > {
    > struct hv_pci_compl *comp_pkt = context;
    >
    > - if (resp_packet_size >= offsetofend(struct pci_response, status))
    > - comp_pkt->completion_status = resp->status;
    > - else
    > - comp_pkt->completion_status = -1;
    > -
    > + comp_pkt->completion_status = resp->status;
    > complete(&comp_pkt->host_event);
    > }
    >
    > @@ -1606,8 +1602,13 @@ static void hv_pci_compose_compl(void *context, struct
    > pci_response *resp,
    > struct pci_create_int_response *int_resp =
    > (struct pci_create_int_response *)resp;
    >
    > + if (resp_packet_size < sizeof(*int_resp)) {
    > + comp_pkt->comp_pkt.completion_status = -1;
    > + goto out;
    > + }
    > comp_pkt->comp_pkt.completion_status = resp->status;
    > comp_pkt->int_desc = int_resp->int_desc;
    > +out:
    > complete(&comp_pkt->comp_pkt.host_event);
    > }
    >
    > @@ -2291,12 +2292,14 @@ static void q_resource_requirements(void *context,
    > struct pci_response *resp,
    > struct q_res_req_compl *completion = context;
    > struct pci_q_res_req_response *q_res_req =
    > (struct pci_q_res_req_response *)resp;
    > + s32 status;
    > int i;
    >
    > - if (resp->status < 0) {
    > + status = (resp_packet_size < sizeof(*q_res_req)) ? -1 : resp->status;
    > + if (status < 0) {
    > dev_err(&completion->hpdev->hbus->hdev->device,
    > "query resource requirements failed: %x\n",
    > - resp->status);
    > + status);
    > } else {
    > for (i = 0; i < PCI_STD_NUM_BARS; i++) {
    > completion->hpdev->probed_bar[i] =
    > @@ -2848,7 +2851,8 @@ static void hv_pci_onchannelcallback(void *context)
    > case PCI_BUS_RELATIONS:
    >
    > bus_rel = (struct pci_bus_relations *)buffer;
    > - if (bytes_recvd <
    > + if (bytes_recvd < sizeof(*bus_rel) ||
    > + bytes_recvd <
    > struct_size(bus_rel, func,
    > bus_rel->device_count)) {
    > dev_err(&hbus->hdev->device,
    > @@ -2862,7 +2866,8 @@ static void hv_pci_onchannelcallback(void *context)
    > case PCI_BUS_RELATIONS2:
    >
    > bus_rel2 = (struct pci_bus_relations2 *)buffer;
    > - if (bytes_recvd <
    > + if (bytes_recvd < sizeof(*bus_rel2) ||
    > + bytes_recvd <
    > struct_size(bus_rel2, func,
    > bus_rel2->device_count)) {
    > dev_err(&hbus->hdev->device,
    > @@ -2876,6 +2881,11 @@ static void hv_pci_onchannelcallback(void *context)
    > case PCI_EJECT:
    >
    > dev_message = (struct pci_dev_incoming *)buffer;
    > + if (bytes_recvd < sizeof(*dev_message)) {
    > + dev_err(&hbus->hdev->device,
    > + "eject message too small\n");
    > + break;
    > + }
    > hpdev = get_pcichild_wslot(hbus,
    > dev_message->wslot.slot);
    > if (hpdev) {
    > @@ -2887,6 +2897,11 @@ static void hv_pci_onchannelcallback(void *context)
    > case PCI_INVALIDATE_BLOCK:
    >
    > inval = (struct pci_dev_inval_block *)buffer;
    > + if (bytes_recvd < sizeof(*inval)) {
    > + dev_err(&hbus->hdev->device,
    > + "invalidate message too small\n");
    > + break;
    > + }
    > hpdev = get_pcichild_wslot(hbus,
    > inval->wslot.slot);
    > if (hpdev) {
    > --
    > 2.25.1

    Reviewed-by: Michael Kelley <mikelley@microsoft.com>

    \
     
     \ /
      Last update: 2022-05-12 17:08    [W:2.910 / U:1.404 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site