| Message ID | 20220504125039.2598-3-parri.andrea@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: hv: (More) Hardening changes | expand |
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, May 4, 2022 5:51 AM > > [ Similarly to commit a765ed47e4516 ("PCI: hv: Fix synchronization > between channel callback and hv_compose_msi_msg()"): ] > > The (on-stack) teardown packet becomes invalid once the completion > timeout in hv_pci_bus_exit() has expired and hv_pci_bus_exit() has > returned. Prevent the channel callback from accessing the invalid > packet by removing the ID associated to such packet from the VMbus > requestor in hv_pci_bus_exit(). > > Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com> > --- > drivers/pci/controller/pci-hyperv.c | 26 +++++++++++++++++++------- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c > index 9a3e17b682eb7..db4b3f86726b2 100644 > --- a/drivers/pci/controller/pci-hyperv.c > +++ b/drivers/pci/controller/pci-hyperv.c > @@ -3620,6 +3620,7 @@ static int hv_pci_probe(struct hv_device *hdev, > static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs) > { > struct hv_pcibus_device *hbus = hv_get_drvdata(hdev); > + struct vmbus_channel *chan = hdev->channel; > struct { > struct pci_packet teardown_packet; > u8 buffer[sizeof(struct pci_message)]; > @@ -3627,13 +3628,14 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool > keep_devs) > struct hv_pci_compl comp_pkt; > struct hv_pci_dev *hpdev, *tmp; > unsigned long flags; > + u64 trans_id; > int ret; > > /* > * After the host sends the RESCIND_CHANNEL message, it doesn't > * access the per-channel ringbuffer any longer. > */ > - if (hdev->channel->rescind) > + if (chan->rescind) > return 0; > > if (!keep_devs) { > @@ -3670,16 +3672,26 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool > keep_devs) > pkt.teardown_packet.compl_ctxt = &comp_pkt; > pkt.teardown_packet.message[0].type = PCI_BUS_D0EXIT; > > - ret = vmbus_sendpacket(hdev->channel, &pkt.teardown_packet.message, > - sizeof(struct pci_message), > - (unsigned long)&pkt.teardown_packet, > - VM_PKT_DATA_INBAND, > - VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); > + ret = vmbus_sendpacket_getid(chan, &pkt.teardown_packet.message, > + sizeof(struct pci_message), > + (unsigned long)&pkt.teardown_packet, > + &trans_id, VM_PKT_DATA_INBAND, > + > VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); > if (ret) > return ret; > > - if (wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ) == 0) > + if (wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ) == 0) { > + /* > + * The completion packet on the stack becomes invalid after > + * 'return'; remove the ID from the VMbus requestor if the > + * identifier is still mapped to/associated with the packet. > + * > + * Cf. hv_pci_onchannelcallback(). > + */ > + vmbus_request_addr_match(chan, trans_id, > + (unsigned long)&pkt.teardown_packet); > return -ETIMEDOUT; > + } > > return 0; > } > -- > 2.25.1 Reviewed-by: Michael Kelley <mikelley@microsoft.com>
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index 9a3e17b682eb7..db4b3f86726b2 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -3620,6 +3620,7 @@ static int hv_pci_probe(struct hv_device *hdev, static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs) { struct hv_pcibus_device *hbus = hv_get_drvdata(hdev); + struct vmbus_channel *chan = hdev->channel; struct { struct pci_packet teardown_packet; u8 buffer[sizeof(struct pci_message)]; @@ -3627,13 +3628,14 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs) struct hv_pci_compl comp_pkt; struct hv_pci_dev *hpdev, *tmp; unsigned long flags; + u64 trans_id; int ret; /* * After the host sends the RESCIND_CHANNEL message, it doesn't * access the per-channel ringbuffer any longer. */ - if (hdev->channel->rescind) + if (chan->rescind) return 0; if (!keep_devs) { @@ -3670,16 +3672,26 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs) pkt.teardown_packet.compl_ctxt = &comp_pkt; pkt.teardown_packet.message[0].type = PCI_BUS_D0EXIT; - ret = vmbus_sendpacket(hdev->channel, &pkt.teardown_packet.message, - sizeof(struct pci_message), - (unsigned long)&pkt.teardown_packet, - VM_PKT_DATA_INBAND, - VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); + ret = vmbus_sendpacket_getid(chan, &pkt.teardown_packet.message, + sizeof(struct pci_message), + (unsigned long)&pkt.teardown_packet, + &trans_id, VM_PKT_DATA_INBAND, + VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); if (ret) return ret; - if (wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ) == 0) + if (wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ) == 0) { + /* + * The completion packet on the stack becomes invalid after + * 'return'; remove the ID from the VMbus requestor if the + * identifier is still mapped to/associated with the packet. + * + * Cf. hv_pci_onchannelcallback(). + */ + vmbus_request_addr_match(chan, trans_id, + (unsigned long)&pkt.teardown_packet); return -ETIMEDOUT; + } return 0; }
[ Similarly to commit a765ed47e4516 ("PCI: hv: Fix synchronization between channel callback and hv_compose_msi_msg()"): ] The (on-stack) teardown packet becomes invalid once the completion timeout in hv_pci_bus_exit() has expired and hv_pci_bus_exit() has returned. Prevent the channel callback from accessing the invalid packet by removing the ID associated to such packet from the VMbus requestor in hv_pci_bus_exit(). Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com> --- drivers/pci/controller/pci-hyperv.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)